From f841f49fb2c4fc93dae5245c5c3d4de0680f318d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 13:20:46 +0700 Subject: [PATCH 01/24] feat(linux): keep stored files before a format upgrade and open .sql files --- .github/workflows/build-linux.yml | 12 + .github/workflows/release-linux.yml | 3 + core/crates/storage/src/connections/mod.rs | 3 +- core/crates/storage/src/document.rs | 12 + core/crates/storage/src/lib.rs | 1 + core/crates/storage/src/query_history/mod.rs | 193 +++++- core/crates/storage/src/upgrade_copies.rs | 549 ++++++++++++++++++ .../0011-copies-before-a-format-upgrade.md | 37 ++ core/docs/decisions/README.md | 1 + linux/ARCHITECTURE.md | 2 +- linux/README.md | 23 +- .../crates/app/src/application/open_files.rs | 49 +- linux/crates/app/src/application/tests.rs | 24 + .../crates/app/src/persistence/state_file.rs | 49 +- .../app/src/services/file_resolution.rs | 179 +++++- .../app/src/services/history_availability.rs | 13 +- linux/crates/app/src/ui/editor/mod.rs | 1 + linux/crates/app/src/ui/import_preview.rs | 116 +++- linux/crates/app/src/ui/session_window/imp.rs | 3 + linux/crates/app/src/ui/session_window/mod.rs | 161 ++++- .../app/src/ui/session_window/picker_page.rs | 13 +- .../ui/session_window/session_window_input.rs | 8 + .../src/ui/session_window/tests/open_file.rs | 204 ++++++- linux/crates/app/src/ui/workspace/mod.rs | 15 + .../app/src/ui/workspace/workspace_tabs.rs | 30 + .../app/src/workspace/workspace_store.rs | 57 +- linux/crates/app/tests/desktop_entry.rs | 47 +- .../data/app.tablepro.TablePro.desktop.in.in | 2 +- .../app.tablepro.TablePro.metainfo.xml.in.in | 2 + linux/data/resources/ui/picker-page.ui | 6 + linux/data/tablepro.1 | 4 +- linux/docs/releasing.md | 2 +- linux/docs/state-management.md | 2 +- linux/docs/storage.md | 58 +- linux/docs/troubleshooting.md | 40 +- linux/po/README.md | 30 +- linux/po/check-complete.sh | 54 ++ 37 files changed, 1933 insertions(+), 72 deletions(-) create mode 100644 core/crates/storage/src/upgrade_copies.rs create mode 100644 core/docs/decisions/0011-copies-before-a-format-upgrade.md create mode 100755 linux/po/check-complete.sh diff --git a/.github/workflows/build-linux.yml b/.github/workflows/build-linux.yml index df1f71f35b..d6358d253e 100644 --- a/.github/workflows/build-linux.yml +++ b/.github/workflows/build-linux.yml @@ -366,6 +366,18 @@ jobs: # checks in every rust-format entry. - name: Check the catalogues run: meson test -C _build --no-rebuild --suite po --print-errorlogs + # Reported, never failed: strings land before the translation run + # after the string freeze. A release refuses the gap + # (release-linux.yml, po/README.md). + - name: Report untranslated entries + run: | + status=0 + po/check-complete.sh >> "$GITHUB_STEP_SUMMARY" || status=$? + case "$status" in + 0) ;; + 1) echo "::warning title=Translations::a language in po/LINGUAS has untranslated or fuzzy entries, which a release refuses (linux/po/README.md)" ;; + *) exit "$status" ;; + esac # Translators start from this template (po/README.md), since few # distributions ship gettext 1.0 yet. - name: Publish tablepro.pot diff --git a/.github/workflows/release-linux.yml b/.github/workflows/release-linux.yml index 83a6ed42d8..3a10dd149a 100644 --- a/.github/workflows/release-linux.yml +++ b/.github/workflows/release-linux.yml @@ -124,6 +124,9 @@ jobs: run: | meson setup _build --fatal-meson-warnings meson compile -C _build tablepro-pot + # A language in po/LINGUAS ships complete (po/README.md). + - name: Every shipped catalogue is complete + run: po/check-complete.sh >> "$GITHUB_STEP_SUMMARY" - uses: actions/upload-artifact@cf430e030ddbb5b0abf93d22962f4752f3646cd9 # v7.0.2 with: name: linux-pot diff --git a/core/crates/storage/src/connections/mod.rs b/core/crates/storage/src/connections/mod.rs index c9abdf2d0d..8631430902 100644 --- a/core/crates/storage/src/connections/mod.rs +++ b/core/crates/storage/src/connections/mod.rs @@ -867,8 +867,7 @@ impl ConnectionStore { fn commit(&self, state: &mut StoreState, mut document: ConnectionsDocument) -> Result<(), StorageError> { // A connection saved with a tag deleted meanwhile drops it here. document.tidy(); - let bytes = crate::document::encode_document(&document)?; - crate::fs::write_private_blocking(&self.inner.path, &bytes)?; + crate::document::write_document_blocking(&self.inner.path, &document)?; *state = StoreState::Ready(document); self.publish(state); Ok(()) diff --git a/core/crates/storage/src/document.rs b/core/crates/storage/src/document.rs index 55ad63551f..271622bd58 100644 --- a/core/crates/storage/src/document.rs +++ b/core/crates/storage/src/document.rs @@ -5,6 +5,7 @@ use serde::de::DeserializeOwned; use serde_json::Value; use crate::document_problem::{DocumentProblem, DocumentProblemKind}; +use crate::error::StorageError; /// One step of a document's upgrade chain: a file at one version /// rewritten into the shape of the next. Its `version` field is left for @@ -97,6 +98,17 @@ fn corrupt(path: &Path, error: &serde_json::Error) -> DocumentProblem { ) } +pub(crate) fn stored_version(bytes: &[u8]) -> Option { + serde_json::from_slice::(bytes).ok()?.version +} + +// A file an older build wrote is copied aside first, and a failed copy leaves it as it was. +pub fn write_document_blocking(path: &Path, document: &D) -> Result<(), StorageError> { + let bytes = encode_document(document)?; + crate::upgrade_copies::keep_before_write_blocking::(path)?; + crate::fs::write_private_blocking(path, &bytes) +} + pub fn encode_document(document: &D) -> Result, serde_json::Error> { #[derive(Serialize)] struct Versioned<'a, D> { diff --git a/core/crates/storage/src/lib.rs b/core/crates/storage/src/lib.rs index ff51b0be62..c0404bb9ac 100644 --- a/core/crates/storage/src/lib.rs +++ b/core/crates/storage/src/lib.rs @@ -21,6 +21,7 @@ pub mod query_history; pub mod saved_queries; mod tag_name_error; mod unix_time; +pub mod upgrade_copies; pub use connection_color::ConnectionColor; pub use connection_group::ConnectionGroup; diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index 0dff0d141d..f450ab171b 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -16,7 +16,7 @@ use std::path::{Path, PathBuf}; use std::time::SystemTime; use sqlx::sqlite::{SqliteConnectOptions, SqlitePoolOptions}; -use sqlx::{AssertSqlSafe, ConnectOptions, Row, SqlitePool}; +use sqlx::{AssertSqlSafe, ConnectOptions, Connection, Row, SqlitePool}; use uuid::Uuid; use crate::error::StorageError; @@ -83,6 +83,9 @@ impl QueryHistory { .max_connections(1) .connect_with(options) .await?; + if let Some(version) = crate::upgrade_copies::pending_upgrade(&pool, &schema::MIGRATOR).await? { + crate::upgrade_copies::keep_database(&pool, &path, version).await?; + } schema::migrate(&pool).await?; Ok(Self { pool, path }) } @@ -302,6 +305,13 @@ impl QueryHistory { .execute(&self.pool) .await? .rows_affected(); + // The delete triggers leave the queries' words in the index until it is optimized. + if affected > 0 { + sqlx::query("INSERT INTO history_search(history_search) VALUES('optimize')") + .execute(&self.pool) + .await?; + } + self.delete_from_upgrade_copies(None).await?; Ok(affected as usize) } @@ -344,12 +354,29 @@ impl QueryHistory { } /// How many queries ran on a connection, pinned ones included. + // The most any one place holds, the copies an upgrade kept included, so queries that outlived the + // retention window only in a copy can still go with the connection. pub async fn count_for_connection(&self, connection: Uuid) -> Result { let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM history WHERE connection_id = ?") .bind(connection.to_string()) .fetch_one(&self.pool) .await?; - Ok(u64::try_from(count).unwrap_or(0)) + let mut most = u64::try_from(count).unwrap_or(0); + for copy in crate::upgrade_copies::copies_blocking(&self.path)? { + let mut database = SqliteConnectOptions::new() + .filename(©.path) + .read_only(true) + .disable_statement_logging() + .connect() + .await?; + let kept: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM history WHERE connection_id = ?") + .bind(connection.to_string()) + .fetch_one(&mut database) + .await?; + database.close().await?; + most = most.max(u64::try_from(kept).unwrap_or(0)); + } + Ok(most) } /// Drop every query run on a connection, pinned ones included, for a @@ -367,9 +394,40 @@ impl QueryHistory { .execute(&self.pool) .await?; } + self.delete_from_upgrade_copies(Some(connection)).await?; Ok(removed) } + // What the user asked to be gone goes from the copies too: each full-text index is optimized, so + // no deleted word stays in its blocks, and VACUUM leaves none in free pages. + async fn delete_from_upgrade_copies(&self, connection: Option) -> Result<(), StorageError> { + for copy in crate::upgrade_copies::copies_blocking(&self.path)? { + let mut database = SqliteConnectOptions::new() + .filename(©.path) + .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .disable_statement_logging() + .connect() + .await?; + let delete = match connection { + Some(connection) => { + sqlx::query("DELETE FROM history WHERE connection_id = ?").bind(connection.to_string()) + } + None => sqlx::query("DELETE FROM history"), + }; + delete.execute(&mut database).await?; + for index in full_text_indexes(&mut database).await? { + sqlx::query(AssertSqlSafe(format!( + "INSERT INTO \"{index}\"(\"{index}\") VALUES('optimize')" + ))) + .execute(&mut database) + .await?; + } + sqlx::query("VACUUM").execute(&mut database).await?; + database.close().await?; + } + Ok(()) + } + /// Every connection the history holds entries for, under the name /// of its latest run, so a renamed connection is listed once. pub async fn connections_with_history(&self) -> Result, StorageError> { @@ -421,6 +479,16 @@ impl QueryHistory { } } +// A copy holds the schema of the build that wrote it, so its indexes are found rather than named. +async fn full_text_indexes(database: &mut sqlx::SqliteConnection) -> Result, StorageError> { + let names: Vec = sqlx::query_scalar( + "SELECT name FROM sqlite_master WHERE type = 'table' AND sql LIKE 'CREATE VIRTUAL TABLE%USING fts5%'", + ) + .fetch_all(&mut *database) + .await?; + Ok(names.into_iter().filter(|name| !name.contains('"')).collect()) +} + #[cfg(test)] mod tests { use std::os::unix::fs::PermissionsExt; @@ -1078,4 +1146,125 @@ mod tests { .await; assert!(integrity.is_ok(), "{integrity:?}"); } + + // Rows the history holds before a copy is kept of it, as an upgrade keeps one. + async fn history_with_a_copy(root: &tempfile::TempDir) -> (QueryHistory, PathBuf) { + let history = QueryHistory::open(&paths(root)).await.expect("open"); + for (query, connection) in [("SELECT qqqq", Uuid::nil()), ("SELECT zzzz", Uuid::max())] { + history.record(entry(query, connection)).await.expect("record"); + } + let copy = crate::upgrade_copies::keep_database(&history.pool, &history.path, 1) + .await + .expect("a copy"); + (history, copy) + } + + async fn copy_rows(copy: &Path) -> i64 { + let mut database = SqliteConnectOptions::new() + .filename(copy) + .read_only(true) + .connect() + .await + .expect("open the copy"); + let rows = sqlx::query_scalar("SELECT COUNT(*) FROM history") + .fetch_one(&mut database) + .await + .expect("count"); + database.close().await.expect("close"); + rows + } + + // Whether a trigram is still in the blocks of a full-text index of the database. + async fn indexed(copy: &Path, trigram: &str) -> bool { + let mut database = SqliteConnectOptions::new() + .filename(copy) + .read_only(true) + .connect() + .await + .expect("open the copy"); + let mut found = false; + for index in full_text_indexes(&mut database).await.expect("the indexes") { + let blocks: i64 = sqlx::query_scalar(AssertSqlSafe(format!( + "SELECT COUNT(*) FROM \"{index}_data\" WHERE instr(block, ?) > 0" + ))) + .bind(trigram.as_bytes()) + .fetch_one(&mut database) + .await + .expect("read the index"); + found |= blocks > 0; + } + database.close().await.expect("close"); + found + } + + #[tokio::test] + async fn deleting_a_connections_history_deletes_it_from_the_copies_too() { + let root = tempfile::tempdir().expect("tempdir"); + let (history, copy) = history_with_a_copy(&root).await; + assert!(indexed(©, "zzz").await); + // As retention would: the live rows go and the copy keeps its own. + sqlx::query("DELETE FROM history WHERE connection_id = ?") + .bind(Uuid::max().to_string()) + .execute(&history.pool) + .await + .expect("the live rows expire"); + + assert_eq!( + history.count_for_connection(Uuid::max()).await.expect("count"), + 1, + "the copy's queries were not counted" + ); + history.delete_for_connection(Uuid::max()).await.expect("delete"); + + assert_eq!(copy_rows(©).await, 1); + assert_eq!(history.count_for_connection(Uuid::max()).await.expect("count"), 0); + assert!( + !indexed(©, "zzz").await, + "a deleted query's words stayed in the copy's index" + ); + assert!(indexed(©, "qqq").await, "another connection's query went"); + } + + #[tokio::test] + async fn clearing_the_history_clears_the_copies_and_keeps_their_saved_queries() { + let root = tempfile::tempdir().expect("tempdir"); + let history = QueryHistory::open(&paths(&root)).await.expect("open"); + history.record(entry("SELECT qqqq", Uuid::nil())).await.expect("record"); + history + .saved_queries() + .save(crate::saved_queries::NewSavedQuery { + name: "kept".to_owned(), + query: "SELECT 1".to_owned(), + connection_id: Uuid::nil(), + connection_name: "test".to_owned(), + }) + .await + .expect("a saved query"); + let copy = crate::upgrade_copies::keep_database(&history.pool, &history.path, 1) + .await + .expect("a copy"); + + history.clear_all().await.expect("clear"); + + assert_eq!(copy_rows(©).await, 0); + assert!( + !indexed(©, "qqq").await, + "a cleared query's words stayed in the copy's index" + ); + assert!( + !indexed(&history.path, "qqq").await, + "a cleared query's words stayed in the history's own index" + ); + let mut database = SqliteConnectOptions::new() + .filename(©) + .read_only(true) + .connect() + .await + .expect("open the copy"); + let saved: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM saved_queries") + .fetch_one(&mut database) + .await + .expect("count"); + assert_eq!(saved, 1, "a saved query went with the history"); + } } diff --git a/core/crates/storage/src/upgrade_copies.rs b/core/crates/storage/src/upgrade_copies.rs new file mode 100644 index 0000000000..8906d2c1ae --- /dev/null +++ b/core/crates/storage/src/upgrade_copies.rs @@ -0,0 +1,549 @@ +// Why a file an older build wrote is copied before its first write in a newer format: core ADR 0011. + +use std::io::ErrorKind; +use std::path::{Path, PathBuf}; +use std::time::SystemTime; + +use sqlx::SqlitePool; +use sqlx::migrate::Migrator; + +use crate::document::VersionedDocument; +use crate::error::StorageError; + +pub const KEPT_PER_FILE: usize = 3; + +const MAX_NAME_ATTEMPTS: u32 = 1_000; + +// Named `.v-`, with `-` after it for a second copy in one second. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct UpgradeCopy { + pub path: PathBuf, + pub version: u64, + made_at: u64, + attempt: u32, +} + +// An `Err` means no copy was made, so the write over `path` must not happen. +pub fn keep_before_write_blocking(path: &Path) -> Result, StorageError> { + let bytes = match std::fs::read(path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(None), + Err(source) => { + return Err(StorageError::Io { + path: path.to_owned(), + source, + }); + } + }; + // A file with no version or one past ours is never written over by + // the stores, so only an older one needs its copy. + match crate::document::stored_version(&bytes) { + Some(found) if found < D::VERSION => { + keep_bytes_blocking(path, u64::from(found), &bytes, SystemTime::now()).map(Some) + } + _ => Ok(None), + } +} + +fn keep_bytes_blocking(path: &Path, version: u64, bytes: &[u8], now: SystemTime) -> Result { + if let Some(newest) = copies_blocking(path)?.pop() + && std::fs::read(&newest.path).is_ok_and(|kept| kept == bytes) + { + return Ok(newest.path); + } + let copy = vacant_name(path, version, now)?; + crate::fs::write_private_blocking(©, bytes)?; + prune_blocking(path, ©)?; + Ok(copy) +} + +// The newest migration a database holds when `migrator` has more to apply. A new database, a current +// one and one a newer build wrote, which the migrator refuses, need no copy. +pub(crate) async fn pending_upgrade(pool: &SqlitePool, migrator: &Migrator) -> Result, StorageError> { + let tracked: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = '_sqlx_migrations'") + .fetch_one(pool) + .await?; + if tracked == 0 { + return Ok(None); + } + let applied: Vec = sqlx::query_scalar("SELECT version FROM _sqlx_migrations") + .fetch_all(pool) + .await?; + let known: Vec = migrator + .iter() + .filter(|migration| migration.migration_type.is_up_migration()) + .map(|migration| migration.version) + .collect(); + let Some(newest) = applied.iter().max() else { + return Ok(None); + }; + let from_newer_build = applied.iter().any(|version| !known.contains(version)); + let behind = known.iter().any(|version| !applied.contains(version)); + Ok((behind && !from_newer_build) + .then(|| u64::try_from(*newest).ok()) + .flatten()) +} + +// Through the connection, since a copy of the file alone leaves out what the WAL still holds. +pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) -> Result { + let copy = vacant_name(path, version, SystemTime::now())?; + let partial = partial_name(©); + remove_if_present(&partial)?; + // VACUUM INTO refuses a file that is not empty, and creates a missing + // one from the umask: made here, it is 0600 like the database. + crate::fs::create_private_file_if_missing(&partial)?; + let target = partial.to_str().ok_or_else(|| StorageError::Write { + path: partial.clone(), + source: std::io::Error::new(ErrorKind::InvalidInput, "the path is not UTF-8"), + })?; + if let Err(error) = sqlx::query("VACUUM INTO ?").bind(target).execute(pool).await { + let _ = std::fs::remove_file(&partial); + return Err(StorageError::Write { + path: copy, + source: database_io_error(&error), + }); + } + // Renamed into place only once on disk, so a copy cut short by a + // power cut never carries a name that says it is whole. + let write_error = |source| StorageError::Write { + path: copy.clone(), + source, + }; + // Windows flushes only a handle opened for writing. + std::fs::OpenOptions::new() + .write(true) + .open(&partial) + .and_then(|file| file.sync_all()) + .map_err(write_error)?; + std::fs::rename(&partial, ©).map_err(write_error)?; + if let Some(directory) = copy.parent() { + tablepro_core::sync_directory(directory).map_err(write_error)?; + } + prune_blocking(path, ©)?; + Ok(copy) +} + +// Oldest first. +pub fn copies_blocking(path: &Path) -> Result, StorageError> { + let Some(name) = path.file_name().map(|name| name.to_string_lossy().into_owned()) else { + return Ok(Vec::new()); + }; + let parent = parent_of(path); + let entries = match std::fs::read_dir(parent) { + Ok(entries) => entries, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(Vec::new()), + Err(source) => { + return Err(StorageError::Io { + path: parent.to_owned(), + source, + }); + } + }; + let mut copies = Vec::new(); + for entry in entries { + let entry = entry.map_err(|source| StorageError::Io { + path: parent.to_owned(), + source, + })?; + if let Some(copy) = parse(&name, &entry.file_name().to_string_lossy(), entry.path()) { + copies.push(copy); + } + } + copies.sort_by_key(|copy| (copy.made_at, copy.attempt)); + Ok(copies) +} + +pub fn remove_all_blocking(path: &Path) -> Result { + let copies = copies_blocking(path)?; + for copy in &copies { + remove_if_present(©.path)?; + } + Ok(copies.len()) +} + +// The copy just made stays even when a clock set back dates it before the others. +fn prune_blocking(path: &Path, made: &Path) -> Result<(), StorageError> { + let older: Vec = copies_blocking(path)? + .into_iter() + .filter(|copy| copy.path != made) + .collect(); + let surplus = older.len().saturating_sub(KEPT_PER_FILE - 1); + for copy in older.iter().take(surplus) { + remove_if_present(©.path)?; + } + Ok(()) +} + +fn parse(name: &str, candidate: &str, path: PathBuf) -> Option { + let rest = candidate.strip_prefix(name)?.strip_prefix(".v")?; + let (version, stamp) = rest.split_once('-')?; + let (made_at, attempt) = match stamp.split_once('-') { + Some((made_at, attempt)) => (made_at, attempt.parse().ok()?), + None => (stamp, 0), + }; + let digits = |text: &str| !text.is_empty() && text.bytes().all(|byte| byte.is_ascii_digit()); + if !digits(version) || !digits(made_at) { + return None; + } + Some(UpgradeCopy { + path, + version: version.parse().ok()?, + made_at: made_at.parse().ok()?, + attempt, + }) +} + +fn vacant_name(path: &Path, version: u64, now: SystemTime) -> Result { + let seconds = now + .duration_since(std::time::UNIX_EPOCH) + .map(|elapsed| elapsed.as_secs()) + .unwrap_or(0); + let name = path + .file_name() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| "file".to_owned()); + let parent = parent_of(path); + for attempt in 0..MAX_NAME_ATTEMPTS { + let candidate = match attempt { + 0 => parent.join(format!("{name}.v{version}-{seconds}")), + _ => parent.join(format!("{name}.v{version}-{seconds}-{attempt}")), + }; + match std::fs::symlink_metadata(&candidate) { + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(candidate), + Ok(_) => continue, + Err(source) => { + return Err(StorageError::Io { + path: candidate, + source, + }); + } + } + } + Err(StorageError::Io { + path: path.to_owned(), + source: std::io::Error::new(ErrorKind::AlreadyExists, "no free name for a copy"), + }) +} + +fn partial_name(copy: &Path) -> PathBuf { + let name = copy.file_name().map(|name| name.to_string_lossy()).unwrap_or_default(); + parent_of(copy).join(format!(".{name}.partial")) +} + +fn parent_of(path: &Path) -> &Path { + match path.parent() { + Some(parent) if !parent.as_os_str().is_empty() => parent, + _ => Path::new("."), + } +} + +fn remove_if_present(path: &Path) -> Result<(), StorageError> { + match std::fs::remove_file(path) { + Err(error) if error.kind() != ErrorKind::NotFound => Err(StorageError::Io { + path: path.to_owned(), + source: error, + }), + _ => Ok(()), + } +} + +// SQLITE_FULL is the one code the app words differently: a full disk. +fn database_io_error(error: &sqlx::Error) -> std::io::Error { + let full = error + .as_database_error() + .and_then(|database| database.code()) + .is_some_and(|code| code == "13"); + match full { + true => std::io::Error::from(ErrorKind::StorageFull), + false => std::io::Error::other(error.to_string()), + } +} + +#[cfg(test)] +#[cfg(unix)] +mod tests { + use std::os::unix::fs::PermissionsExt; + use std::time::{Duration, UNIX_EPOCH}; + + use serde::{Deserialize, Serialize}; + + use super::*; + + #[derive(Debug, Default, Serialize, Deserialize)] + struct Sample { + items: Vec, + } + + impl VersionedDocument for Sample { + const KIND: &'static str = "sample"; + const VERSION: u32 = 3; + } + + fn at(seconds: u64) -> SystemTime { + UNIX_EPOCH + Duration::from_secs(seconds) + } + + fn names(dir: &Path) -> Vec { + let mut names: Vec = std::fs::read_dir(dir) + .expect("list") + .map(|entry| entry.expect("entry").file_name().to_string_lossy().into_owned()) + .collect(); + names.sort(); + names + } + + #[derive(Debug, Default, Serialize, Deserialize)] + struct Bumped { + items: Vec, + } + + fn same_shape(document: serde_json::Value) -> Result { + Ok(document) + } + + // A format bumped once, so the write path can be tested without any format the app shipped. + impl VersionedDocument for Bumped { + const KIND: &'static str = "bumped"; + const VERSION: u32 = 2; + const UPGRADES: &'static [crate::document::Upgrade] = &[same_shape]; + } + + #[test] + fn the_first_write_over_an_older_file_keeps_it_and_later_writes_do_not() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("bumped.json"); + let older = br#"{"version": 1, "items": ["a"]}"#; + std::fs::write(&file, older).expect("seed"); + let read: Bumped = + crate::document::decode_document(&file, &std::fs::read(&file).expect("read")).expect("upgraded"); + + crate::document::write_document_blocking(&file, &read).expect("first write"); + crate::document::write_document_blocking( + &file, + &Bumped { + items: vec!["b".to_owned()], + }, + ) + .expect("second write"); + + let copies = copies_blocking(&file).expect("copies"); + assert_eq!(copies.len(), 1, "{copies:?}"); + assert_eq!(copies[0].version, 1); + assert_eq!(std::fs::read(&copies[0].path).expect("read"), older); + let written = std::fs::read_to_string(&file).expect("read"); + assert!( + written.contains("\"version\": 2") && written.contains("\"b\""), + "{written}" + ); + } + + fn migration(version: i64, sql: &'static str) -> sqlx::migrate::Migration { + sqlx::migrate::Migration::new( + version, + format!("step {version}").into(), + sqlx::migrate::MigrationType::Simple, + sqlx::SqlSafeStr::into_sql_str(sql), + false, + ) + } + + fn base() -> Migrator { + Migrator::with_migrations(vec![migration( + 1, + "CREATE TABLE items (id INTEGER PRIMARY KEY, name TEXT)", + )]) + } + + fn bumped() -> Migrator { + Migrator::with_migrations(vec![ + migration(1, "CREATE TABLE items (id INTEGER PRIMARY KEY, name TEXT)"), + migration(2, "ALTER TABLE items ADD COLUMN size INTEGER"), + ]) + } + + async fn database(path: &Path) -> SqlitePool { + crate::fs::create_private_file_if_missing(path).expect("the file"); + let options = sqlx::sqlite::SqliteConnectOptions::new() + .filename(path) + .journal_mode(sqlx::sqlite::SqliteJournalMode::Wal); + sqlx::sqlite::SqlitePoolOptions::new() + .max_connections(1) + .connect_with(options) + .await + .expect("connect") + } + + #[tokio::test] + async fn a_database_behind_its_migrations_is_copied_with_what_its_wal_holds() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("items.db"); + let pool = database(&path).await; + base().run(&pool).await.expect("the older schema"); + sqlx::query("INSERT INTO items (name) VALUES ('kept')") + .execute(&pool) + .await + .expect("a row, still in the WAL"); + + let version = pending_upgrade(&pool, &bumped()) + .await + .expect("checked") + .expect("behind"); + let copy = keep_database(&pool, &path, version).await.expect("a copy"); + + assert_eq!(version, 1); + assert_eq!( + copy.file_name().expect("name").to_string_lossy().split('-').next(), + Some("items.db.v1") + ); + assert_eq!( + std::fs::metadata(©).expect("metadata").permissions().mode() & 0o777, + 0o600 + ); + let copied = database(©).await; + let rows: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM items") + .fetch_one(&copied) + .await + .expect("count"); + assert_eq!(rows, 1, "the copy missed what the WAL held"); + assert_eq!( + names(root.path()) + .iter() + .filter(|name| name.ends_with(".partial")) + .count(), + 0 + ); + } + + #[tokio::test] + async fn a_new_current_or_newer_database_needs_no_copy() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("items.db"); + let pool = database(&path).await; + + assert_eq!(pending_upgrade(&pool, &bumped()).await.expect("new"), None); + bumped().run(&pool).await.expect("current"); + assert_eq!(pending_upgrade(&pool, &bumped()).await.expect("current"), None); + assert_eq!(pending_upgrade(&pool, &base()).await.expect("newer"), None); + } + + #[test] + fn an_older_file_is_copied_byte_for_byte_and_private() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("sample.json"); + let older = br#"{"version": 2, "items": ["a"]}"#; + std::fs::write(&file, older).expect("seed"); + + let copy = keep_before_write_blocking::(&file) + .expect("keep") + .expect("a copy"); + + assert_eq!(std::fs::read(©).expect("read"), older); + assert_eq!(std::fs::read(&file).expect("read"), older, "the file itself changed"); + assert_eq!( + std::fs::metadata(©).expect("metadata").permissions().mode() & 0o777, + 0o600 + ); + let copies = copies_blocking(&file).expect("copies"); + assert_eq!(copies.len(), 1); + assert_eq!(copies[0].version, 2); + assert!( + copy.file_name() + .expect("name") + .to_string_lossy() + .starts_with("sample.json.v2-") + ); + } + + #[test] + fn a_current_newer_missing_or_unversioned_file_is_not_copied() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("sample.json"); + + assert_eq!(keep_before_write_blocking::(&file).expect("missing"), None); + for bytes in [ + &br#"{"version": 3, "items": []}"#[..], + br#"{"version": 4, "items": []}"#, + br#"{"items": []}"#, + b"not json", + ] { + std::fs::write(&file, bytes).expect("seed"); + assert_eq!(keep_before_write_blocking::(&file).expect("keep"), None); + } + + assert_eq!(names(root.path()), ["sample.json"]); + } + + #[test] + fn the_same_bytes_are_kept_once() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("sample.json"); + std::fs::write(&file, br#"{"version": 1, "names": []}"#).expect("seed"); + + let first = keep_bytes_blocking(&file, 1, b"same", at(100)).expect("first"); + let second = keep_bytes_blocking(&file, 1, b"same", at(200)).expect("second"); + + assert_eq!(first, second); + assert_eq!(copies_blocking(&file).expect("copies").len(), 1); + } + + #[test] + fn only_the_newest_copies_are_kept() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("sample.json"); + + for (seconds, bytes) in [(100, "a"), (300, "c"), (200, "b"), (400, "d"), (400, "e")] { + keep_bytes_blocking(&file, 2, bytes.as_bytes(), at(seconds)).expect("keep"); + } + + let kept: Vec> = copies_blocking(&file) + .expect("copies") + .iter() + .map(|copy| std::fs::read(©.path).expect("read")) + .collect(); + assert_eq!(kept, [b"c".to_vec(), b"d".to_vec(), b"e".to_vec()]); + assert_eq!( + names(root.path()), + ["sample.json.v2-300", "sample.json.v2-400", "sample.json.v2-400-1"] + ); + } + + #[test] + fn a_copy_dated_before_the_others_is_kept() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("sample.json"); + for (seconds, bytes) in [(500, "a"), (600, "b"), (700, "c")] { + keep_bytes_blocking(&file, 2, bytes.as_bytes(), at(seconds)).expect("keep"); + } + + let made = keep_bytes_blocking(&file, 2, b"d", at(100)).expect("keep"); + + assert_eq!(std::fs::read(&made).expect("the new copy"), b"d"); + assert_eq!( + names(root.path()), + ["sample.json.v2-100", "sample.json.v2-600", "sample.json.v2-700"] + ); + } + + #[test] + fn only_that_files_copies_are_counted_and_removed() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("a.json"); + let other = root.path().join("a.json2"); + keep_bytes_blocking(&file, 1, b"a", at(100)).expect("keep"); + keep_bytes_blocking(&other, 1, b"b", at(100)).expect("keep"); + for unrelated in ["a.json.corrupt-100", "a.json.v1-", "a.json.vx-100", "a.json.v1-100-x"] { + std::fs::write(root.path().join(unrelated), b"x").expect("seed"); + } + + assert_eq!(copies_blocking(&file).expect("copies").len(), 1); + assert_eq!(remove_all_blocking(&file).expect("remove"), 1); + + assert!(copies_blocking(&file).expect("copies").is_empty()); + assert_eq!(copies_blocking(&other).expect("copies").len(), 1); + assert_eq!( + remove_all_blocking(&root.path().join("missing").join("a.json")).expect("no directory"), + 0 + ); + } +} diff --git a/core/docs/decisions/0011-copies-before-a-format-upgrade.md b/core/docs/decisions/0011-copies-before-a-format-upgrade.md new file mode 100644 index 0000000000..9afa18f585 --- /dev/null +++ b/core/docs/decisions/0011-copies-before-a-format-upgrade.md @@ -0,0 +1,37 @@ +# 0011: A file in an older format is copied beside itself before its first write in the newer one + +- **Status**: Accepted +- **Date**: 2026-11-05 + +## Context + +Every file `tablepro-storage` owns carries its format version: a JSON document its `version` field (`VersionedDocument`), the query history database its sqlx migrations. A newer build reads an older file through upgrade steps in memory, and a build refuses a file whose version it does not know. The next save then wrote the upgraded file straight over the old one: opening a saved connection stamps its last-opened time, so a plain open after an update rewrote `connections.json`, and `QueryHistory::open` migrated `history.db` in place, saved queries included. An atomic write protects a write in progress, not the way back: after an update, the older build refused everything the newer one had written once. + +## Decision + +Before the first write that replaces a file holding an older format, the storage crate keeps a private copy of it beside the file, named `.v-`, keeps the newest three per file, and refuses the write when the copy cannot be made. + +## Rationale + +- **One write path decides it.** `document::write_document_blocking` reads the version of the file on disk and copies an older one before writing, so every store that writes a versioned document gets the copy without remembering what it read. The Linux app's workspace writer, which injects its own write for tests, calls `upgrade_copies::keep_before_write_blocking` the same way. +- **A database is copied through SQLite, not as a file.** `history.db` runs in WAL mode, and a copy of the main file alone misses what the WAL still holds ([How To Corrupt An SQLite Database File](https://www.sqlite.org/howtocorrupt.html), 1.2 and 1.4). `VACUUM INTO` through the open connection writes a consistent snapshot ([VACUUM](https://www.sqlite.org/lang_vacuum.html#vacuuminto)) before any migration runs. It goes to a `.partial` file created 0600, is synced and renamed into place, so a copy cut short never carries a finished name. +- **Beside the file, like the copies a Reset moves aside.** `connections.json.v1-…` sits next to `connections.json.corrupt-…`, a restore is a rename in one folder, and a session's purge removes its copies with its file. +- **Three per file, as Chromium keeps three user-data snapshots** taken at each major update for a rollback ([Keep data during version rollback](https://support.google.com/chrome/a/answer/9917429)). A copy whose bytes match the newest one is not made again. +- **No copy, no write.** A failed copy fails the write, the file stays as it was, and the app words the error by the copy's path like any other failed write. Upgrading without the way back is what this decision removes. +- **What the user deletes is deleted from the copies.** Clear History and deleting a connection's history also delete those rows from each history copy, then `VACUUM` it; saved queries stay. + +## Consequences + +- An older build reads the copy of its own format once the user puts it back (`linux/docs/troubleshooting.md`, After a downgrade). Nothing restores a copy on its own. +- A copy holds what the file held at the upgrade, including connections deleted since. It is as private as the file: 0600 in the app's 0700 folders. +- A history copy is as large as the database after `VACUUM`, and three of them can sit beside it. +- Reading never makes a copy, so a newer build that only looks and is then removed leaves nothing behind. +- The Windows and macOS apps get the same copies through `ConnectionStore`, under their own storage folders. +- Every stored format starts again at version 1 for the first release, so no copy is made until a later release raises one. The hook is tested with a synthetic version bump of a document and of a migration list. + +## Alternatives considered + +- **Copies in one backup folder.** Easier to list, but every store would need the folder passed in, and a store that forgot would write without a copy. Beside the file needs no wiring. +- **Copy at read time.** Leaves a copy behind for every build that only reads, and the copy would not be the one the write replaced if the file changed in between. +- **Warn and upgrade when the copy fails.** Keeps writes working on a broken folder, but loses the way back silently, which is the failure this decision is about. +- **The SQLite backup API** (`sqlite3_backup`). Uses less CPU and runs in steps, but sqlx does not expose it, and `VACUUM INTO` is one statement that also leaves no deleted content in the copy. diff --git a/core/docs/decisions/README.md b/core/docs/decisions/README.md index 2c3f0fd4c2..eb764be112 100644 --- a/core/docs/decisions/README.md +++ b/core/docs/decisions/README.md @@ -18,3 +18,4 @@ The format, the statuses and how a later ADR changes an earlier one follow [the | [0008](0008-importers-from-other-apps.md) | Importers from other apps live in the core | Accepted | One crate reads each app's files on every platform into one record shape, takes keychains and directories from the app, says what it cannot carry, and reviews the result against the library with the macOS duplicate handling. | | [0009](0009-in-process-ssh-on-windows.md) | SSH on Windows runs in process on russh | Accepted | russh behind `tablepro-ssh`'s `in-process` feature, on every target; Windows uses it, Linux keeps OpenSSH. Forwards are channels with no address, so MySQL and MongoDB relay them through a private listener that serves only this process. | | [0010](0010-macos-on-the-core.md) | The macOS app moves to the core in slices, starting with the connection library | Accepted | UniFFI Swift bindings in an XCFramework of the static library, without the engines, at the app's deployment target; UI, Keychain and the CloudKit transport stay Swift; the core maps the library's CloudKit records both ways with the existing fields and merges a pull field by field. | +| [0011](0011-copies-before-a-format-upgrade.md) | A file in an older format is copied beside itself before its first write in the newer one | Accepted | `write_document_blocking` and the history's open copy a file an older build wrote to `.v-` before replacing it, the history through `VACUUM INTO`; three kept per file; no copy, no write. | diff --git a/linux/ARCHITECTURE.md b/linux/ARCHITECTURE.md index 4991546617..9fb1c83644 100644 --- a/linux/ARCHITECTURE.md +++ b/linux/ARCHITECTURE.md @@ -75,7 +75,7 @@ One second runtime remains. Relm4 runs a component's commands (`sender.command`, A quit freezes the record before it asks the first window, so each window it closes stays on it, and writes it before the process may end. An approved quit then empties the list in memory; a cancelled one keeps only the windows still showing or connecting their session. Shutdown freezes it too: windows that go with the process are not the user closing them. An approved quit can leave the process running while a session closes, and an activate that reaches it then restores the record as a new launch would. -Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. +Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`). It goes to the most recently focused window, or to a new picker, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no session in front, the script waits behind a banner on the picker until the first relational session joins the window; with a key-value or document session in front, it waits in a new picker window. A tab already bound to the file comes forward instead. ## Windows and sessions diff --git a/linux/README.md b/linux/README.md index 0f38337814..560f2a7628 100644 --- a/linux/README.md +++ b/linux/README.md @@ -237,22 +237,27 @@ After a change to `Cargo.lock`, `build-aux/update-third-party-licenses.sh` rewrites `debian/copyright` and the spec's `License` tag along with `data/licenses`. -## Opening database files +## Opening files Once installed, TablePro is in the file manager's **Open With** for -SQLite databases (`application/vnd.sqlite3`). shared-mime-info has no -type for DuckDB files, so a `.duckdb` file opens from the connect dialog -or a terminal: +SQLite databases (`application/vnd.sqlite3`) and SQL files +(`application/sql`). shared-mime-info has no type for DuckDB files, so a +`.duckdb` file opens from the connect dialog or a terminal: ```bash -tablepro app.sqlite3 sales.duckdb +tablepro app.sqlite3 sales.duckdb report.sql ``` The file's own header picks the engine, whatever its name. A running -TablePro opens each file in its own window and the new launch exits. A -file already open brings its window forward. A database on a network or -FUSE filesystem asks whether to open read-only first, because neither -engine's locking can be trusted there. +TablePro opens each database in its own window and the new launch exits. +A database already open brings its window forward. A database on a +network or FUSE filesystem asks whether to open read-only first, because +neither engine's locking can be trusted there. + +A SQL file opens in a new editor tab of the connection in front of the +last window you used, and Save writes back to it. It never runs by +itself. With no SQL connection in front, the window asks you to choose +one first. A file already open in a tab brings that tab forward. ## Vim keybindings diff --git a/linux/crates/app/src/application/open_files.rs b/linux/crates/app/src/application/open_files.rs index 321b4c1e30..b457dab2fc 100644 --- a/linux/crates/app/src/application/open_files.rs +++ b/linux/crates/app/src/application/open_files.rs @@ -7,9 +7,10 @@ use crate::services::file_resolution::{FileTarget, ResolvedFile, resolve_file_bl use crate::ui::{SessionWindow, SessionWindowInput}; impl TableProApplication { - /// Open each file the file manager or the command line hands over: - /// in the window that has it open already, in the active window - /// while it only shows the picker, or in a new window. + /// Open each file the file manager or the command line hands over. + /// A database opens in the window that has it open already, in the + /// active window while it only shows the picker, or in a new window. + /// A SQL script opens in the last window used, or in a new one. pub fn open_files(&self, files: &[gio::File]) { for file in files { self.open_file(file.clone()); @@ -35,9 +36,15 @@ impl TableProApplication { let hold = self.hold(); let app = self.downgrade(); glib::spawn_future_local(async move { - let key = resolving.await.ok().and_then(|resolved| session_key(&resolved)); + let resolved = resolving.await.ok(); if let Some(app) = app.upgrade() { - app.place_file(file, key); + match resolved { + Some(ResolvedFile { + uri, + target: Ok(FileTarget::Script), + }) => app.place_script(gio::File::for_uri(&uri)), + resolved => app.place_file(file, resolved.as_ref().and_then(session_key)), + } } drop(hold); }); @@ -63,6 +70,36 @@ impl TableProApplication { } } } + + // A second tab on one file would have two editors saving over each + // other, so a tab already bound to it comes forward instead. + fn place_script(&self, file: gio::File) { + // GTK lists the windows most recently focused first. + let windows: Vec = self + .windows() + .into_iter() + .filter_map(|window| window.downcast::().ok()) + .collect(); + if let Some((window, tab)) = windows + .iter() + .find_map(|window| Some((window, window.script_tab(&file)?))) + { + window.send(SessionWindowInput::FocusTab(tab)); + window.present(); + return; + } + let window = match windows.into_iter().next() { + Some(window) => Some(window), + None => self.open_window(WindowLaunch::Picker).and_downcast::(), + }; + match window { + Some(window) => { + window.send(SessionWindowInput::OpenScript(file)); + window.present(); + } + None => tracing::warn!("no window to open a SQL file in"), + } + } } /// The session a resolved file opens, so its window can be found. @@ -70,6 +107,6 @@ fn session_key(resolved: &ResolvedFile) -> Option { match &resolved.target { Ok(FileTarget::Saved(saved)) => Some(SessionKey::Saved(saved.id)), Ok(FileTarget::File(identity)) => Some(SessionKey::File(identity.uri.clone())), - Err(_) => None, + Ok(FileTarget::Script) | Err(_) => None, } } diff --git a/linux/crates/app/src/application/tests.rs b/linux/crates/app/src/application/tests.rs index 3156cd9a66..3a0e7880e9 100644 --- a/linux/crates/app/src/application/tests.rs +++ b/linux/crates/app/src/application/tests.rs @@ -394,6 +394,30 @@ fn opening_a_file_held_behind_another_session_brings_that_session_forward() { assert_eq!(test.app.windows().len(), 1, "the file opened another window"); } +#[gtk4::test] +fn a_sql_file_with_no_window_opens_a_picker_that_asks() { + let test = TestApp::started("OpenScriptNoWindow"); + let path = test._backing.dir.path().join("report.sql"); + std::fs::write(&path, "SELECT 1;").expect("write the script"); + + test.app.open(&[gio::File::for_path(&path)], ""); + + let picker = || { + test.app + .windows() + .into_iter() + .find_map(|window| window.downcast::().ok()) + }; + wait_until(WAIT, || { + picker() + .and_then(|window| window.script_banner()) + .is_some_and(|banner| banner.is_revealed()) + }) + .expect("a picker asking for a connection"); + assert_eq!(test.app.windows().len(), 1, "more than the one picker opened"); + assert_eq!(picker().map(|window| window.page_name()), Some("picker")); +} + #[gtk4::test] fn activate_restores_nothing_when_restore_session_false() { let test = TestApp::started("NoRestore"); diff --git a/linux/crates/app/src/persistence/state_file.rs b/linux/crates/app/src/persistence/state_file.rs index 340bf75e8f..0045a2b9dc 100644 --- a/linux/crates/app/src/persistence/state_file.rs +++ b/linux/crates/app/src/persistence/state_file.rs @@ -2,7 +2,7 @@ use std::cell::{Cell, RefCell}; use std::path::PathBuf; use tablepro_session::runtime::{LatestWinsWriter, Tasks}; -use tablepro_storage::document::{VersionedDocument, decode_document, encode_document}; +use tablepro_storage::document::{VersionedDocument, decode_document, write_document_blocking}; use tablepro_storage::{DocumentProblem, DocumentProblemKind}; /// A small versioned JSON file the app keeps in memory and writes back @@ -117,14 +117,7 @@ fn start_over(path: &std::path::Path, problem: DocumentProblem) -> ( } fn write(path: &std::path::Path, document: &D) { - let bytes = match encode_document(document) { - Ok(bytes) => bytes, - Err(error) => { - tracing::warn!(%error, kind = D::KIND, "could not serialise state"); - return; - } - }; - if let Err(error) = tablepro_storage::fs::write_private_blocking(path, &bytes) { + if let Err(error) = write_document_blocking(path, document) { tracing::warn!(%error, kind = D::KIND, "could not save state"); } } @@ -252,6 +245,44 @@ mod tests { assert!(!path.exists(), "a no-op update still wrote the file"); } + #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)] + struct Upgraded { + items: Vec, + } + + fn same_shape(document: serde_json::Value) -> Result { + Ok(document) + } + + impl VersionedDocument for Upgraded { + const KIND: &'static str = "upgraded"; + const VERSION: u32 = 2; + const UPGRADES: &'static [tablepro_storage::document::Upgrade] = &[same_shape]; + } + + #[tokio::test] + async fn an_older_file_is_copied_aside_before_its_first_write() { + let root = tempfile::tempdir().expect("tempdir"); + let original = r#"{"version": 1, "items": ["old"]}"#; + let path = seed(&root, original); + let file = StateFile::::load(path.clone(), &paused_tasks()); + assert_eq!(file.read(|upgraded| upgraded.items.clone()), vec!["old".to_owned()]); + let copies = || tablepro_storage::upgrade_copies::copies_blocking(&path).expect("copies"); + assert!(copies().is_empty(), "a read made a copy"); + + file.update(|upgraded| { + upgraded.items.push("new".to_owned()); + true + }); + file.flush().await; + + let kept = copies(); + assert_eq!(kept.len(), 1, "{kept:?}"); + assert_eq!(std::fs::read_to_string(&kept[0].path).expect("read"), original); + let written = std::fs::read_to_string(&path).expect("read"); + assert!(written.contains("\"version\": 2"), "{written}"); + } + #[tokio::test] async fn a_readable_file_round_trips() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/linux/crates/app/src/services/file_resolution.rs b/linux/crates/app/src/services/file_resolution.rs index 474a36c29b..08fa408a46 100644 --- a/linux/crates/app/src/services/file_resolution.rs +++ b/linux/crates/app/src/services/file_resolution.rs @@ -1,5 +1,6 @@ -//! A database file handed to TablePro, resolved to the session it -//! opens. Resolving reads the disk, so it runs on the blocking pool. +//! A file handed to TablePro, resolved to the session it opens or to +//! the SQL script it is. Resolving reads the disk, so it runs on the +//! blocking pool. use std::io::Read; use std::path::{Path, PathBuf}; @@ -31,6 +32,8 @@ pub enum FileTarget { /// opens, with its name, colour and saved tabs. Saved(Box), File(FileIdentity), + /// A SQL script, which opens in an editor tab rather than connecting. + Script, } /// A file the user opened and what it resolved to. @@ -59,6 +62,17 @@ pub fn resolve_file_blocking( connections: &ConnectionStore, drivers: &DriverRegistry, ) -> ResolvedFile { + if let Some(script) = path.as_deref().filter(|path| is_script_blocking(path, drivers)) { + // One file reached through a link and through its target opens one tab, as a database opens one session. + let uri = std::fs::canonicalize(script) + .ok() + .and_then(|canonical| glib::filename_to_uri(&canonical, None).ok()) + .map_or(uri, |canonical| canonical.to_string()); + return ResolvedFile { + uri, + target: Ok(FileTarget::Script), + }; + } let target = resolve_file_identity_blocking(path.as_deref(), &uri).map(|identity| { let saved = match connections.load_blocking() { Ok(saved) => saved.to_vec(), @@ -83,6 +97,32 @@ pub fn resolve_file_blocking( ResolvedFile { uri, target } } +// The header decides over the name, as it does between engines. A file +// that cannot be read stays a script, so the editor says why. +fn is_script_blocking(path: &Path, drivers: &DriverRegistry) -> bool { + let header = match read_header_blocking(path, drivers.file_header_len()) { + Ok(header) => header, + // An engine here holds it, so it is a database, and reading it as text would drop that engine's locks. + Err(DriverError::Busy(_)) => return false, + Err(_) => Vec::new(), + }; + let (content_type, _) = gio::content_type_guess(Some(path), &header); + // The suffix covers a system without shared-mime-info. + let named_sql = gio::content_type_is_a(&content_type, SQL_CONTENT_TYPE) + || path + .extension() + .and_then(|extension| extension.to_str()) + .is_some_and(|extension| extension.eq_ignore_ascii_case(SQL_SUFFIX)); + named_sql + && !drivers + .iter() + .filter_map(|driver| driver.capabilities().file_format()) + .any(|format| format.signature.matches(&header)) +} + +const SQL_CONTENT_TYPE: &str = "application/sql"; +const SQL_SUFFIX: &str = "sql"; + /// The first `len` bytes of the file, or as many as it has, which say /// which engine opens it. /// @@ -287,4 +327,139 @@ mod tests { resolved.target ); } + + fn resolve_named(name: &str, contents: Option<&[u8]>) -> ResolvedFile { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join(name); + if let Some(contents) = contents { + std::fs::write(&path, contents).expect("seed"); + } + let store = ConnectionStore::new(&tablepro_storage::StoragePaths::under( + root.path(), + "tablepro-test", + "app.tablepro.Test", + )); + let uri = format!("file://{}", path.display()); + resolve_file_blocking(Some(path), uri, &store, &drivers(&FakeDriver::FILE_CAPABILITIES)) + } + + #[test] + fn a_sql_file_is_a_script() { + let resolved = resolve_named("report.sql", Some(b"SELECT 1;\n")); + + assert!( + matches!(resolved.target, Ok(FileTarget::Script)), + "{:?}", + resolved.target + ); + } + + #[test] + fn a_sql_suffix_in_any_case_is_a_script() { + let resolved = resolve_named("REPORT.SQL", Some(b"SELECT 1;\n")); + + assert!( + matches!(resolved.target, Ok(FileTarget::Script)), + "{:?}", + resolved.target + ); + } + + // The engine that reads an empty file as a new database would take it. + #[test] + fn an_empty_sql_file_is_a_script_not_an_empty_database() { + let resolved = resolve_named("new.sql", Some(b"")); + + assert!( + matches!(resolved.target, Ok(FileTarget::Script)), + "{:?}", + resolved.target + ); + } + + #[test] + fn a_database_named_sql_opens_its_engine() { + let signature = FakeDriver::FILE_CAPABILITIES + .file_format() + .expect("a file engine") + .signature; + let mut header = vec![0u8; 64]; + header[signature.offset..signature.header_len()].copy_from_slice(signature.bytes); + + let resolved = resolve_named("backup.sql", Some(&header)); + + assert!( + matches!(resolved.target, Ok(FileTarget::File(_))), + "{:?}", + resolved.target + ); + } + + #[test] + fn a_file_named_sql_this_process_holds_is_not_read_as_a_script() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("backup.sql"); + std::fs::write(&path, b"SELECT 1;").expect("seed"); + let held = std::fs::File::open(&path).expect("hold the file"); + let store = ConnectionStore::new(&tablepro_storage::StoragePaths::under( + root.path(), + "tablepro-test", + "app.tablepro.Test", + )); + + let resolved = resolve_file_blocking( + Some(path.clone()), + format!("file://{}", path.display()), + &store, + &drivers(&FakeDriver::FILE_CAPABILITIES), + ); + + assert!( + matches!(resolved.target, Ok(FileTarget::File(_))), + "{:?}", + resolved.target + ); + drop(held); + } + + #[test] + fn a_script_through_a_link_resolves_to_its_target() { + let root = tempfile::tempdir().expect("tempdir"); + let target = root.path().join("report.sql"); + std::fs::write(&target, b"SELECT 1;").expect("seed"); + let link = root.path().join("link.sql"); + std::os::unix::fs::symlink(&target, &link).expect("link"); + let store = ConnectionStore::new(&tablepro_storage::StoragePaths::under( + root.path(), + "tablepro-test", + "app.tablepro.Test", + )); + + let resolved = resolve_file_blocking( + Some(link.clone()), + format!("file://{}", link.display()), + &store, + &drivers(&FakeDriver::FILE_CAPABILITIES), + ); + + assert!( + matches!(resolved.target, Ok(FileTarget::Script)), + "{:?}", + resolved.target + ); + let canonical = std::fs::canonicalize(&target).expect("canonical"); + assert_eq!(gio::File::for_uri(&resolved.uri).path(), Some(canonical)); + } + + // The editor's read then says why it cannot be opened. + #[test] + fn a_missing_sql_file_is_still_a_script() { + let resolved = resolve_named("gone.sql", None); + + assert!( + matches!(resolved.target, Ok(FileTarget::Script)), + "{:?}", + resolved.target + ); + } } diff --git a/linux/crates/app/src/services/history_availability.rs b/linux/crates/app/src/services/history_availability.rs index 3d9b83254e..9c5abeef39 100644 --- a/linux/crates/app/src/services/history_availability.rs +++ b/linux/crates/app/src/services/history_availability.rs @@ -47,7 +47,9 @@ fn open_failure_text(error: &StorageError) -> String { StorageError::HistoryMigration(_) => { crate::i18n::gettext("The query history could not be updated for this version of TablePro.") } - StorageError::Io { path, source } | StorageError::Permissions { path, source } => { + StorageError::Io { path, source } + | StorageError::Permissions { path, source } + | StorageError::Write { path, source } => { let path = path.display().to_string(); match source.kind() { ErrorKind::PermissionDenied | ErrorKind::ReadOnlyFilesystem => { @@ -90,6 +92,15 @@ mod tests { HistoryAvailability::failed(&StorageError::HistoryNewerThanApp { version: 9 }).failure(), Some("The query history was written by a newer version of TablePro. Update TablePro to read it.") ); + let copy = "/home/ada/.local/state/tablepro/history.db.v20260918000000-1762300000"; + assert_eq!( + HistoryAvailability::failed(&StorageError::Write { + path: PathBuf::from(copy), + source: std::io::Error::from(ErrorKind::StorageFull), + }) + .failure(), + Some(format!("The disk that holds {copy} is full.").as_str()) + ); assert_eq!( HistoryAvailability::failed(&StorageError::NotInitialised).failure(), Some("The query history file could not be read. It may be damaged.") diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index 8a8027f644..380d36f0af 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -58,6 +58,7 @@ use self::results_pane::ResultPage; use self::run_log::{EntryKind, RunEvent, RunLog}; use self::run_scope::ScopedText; use self::script_runner::{RUN_ROW_LIMIT, RunBatch, ScriptRun, plan_batches, run_in_tab}; +pub(crate) use self::sql_file::load_text; pub(crate) use self::transaction_dirty::OpenTransaction; use super::data_grid::{DialectSource, GridRequest}; use super::export::{ExportOffer, ExportOrigin, ExportQuery, ExportSource}; diff --git a/linux/crates/app/src/ui/import_preview.rs b/linux/crates/app/src/ui/import_preview.rs index e7735e7160..6ae3e78179 100644 --- a/linux/crates/app/src/ui/import_preview.rs +++ b/linux/crates/app/src/ui/import_preview.rs @@ -35,12 +35,14 @@ pub(crate) fn import_preview_dialog(preview: &ImportPreview, drivers: &DriverReg for entry in preview.entries() { rows.append(&glib::BoxedAnyObject::new(entry_text(entry, drivers))); } - // A row is built only while it is on screen, so a file of thousands - // of connections opens the dialog as fast as one of three. + // A list view builds only the rows on screen, so a file of thousands of connections opens as fast as three. + // boxed-list styles a GtkListBox only; a card with separators draws the same box. let list = gtk::ListView::builder() .model(>k::NoSelection::new(Some(rows))) .factory(&row_factory()) - .css_classes(["boxed-list"]) + .show_separators(true) + .css_classes(["rich-list", "card"]) + .overflow(gtk::Overflow::Hidden) .build(); let scrolled = gtk::ScrolledWindow::builder() .hscrollbar_policy(gtk::PolicyType::Never) @@ -101,10 +103,8 @@ fn row_factory() -> gtk::SignalListItemFactory { let factory = gtk::SignalListItemFactory::new(); factory.connect_setup(|_, object| { if let Some(item) = object.downcast_ref::() { - // A name and a host are the file's text, never markup. - let row = adw::ActionRow::builder().use_markup(false).build(); item.set_activatable(false); - item.set_child(Some(&row)); + item.set_child(Some(&EntryRow::new().root)); } }); factory.connect_bind(|_, object| { @@ -112,18 +112,65 @@ fn row_factory() -> gtk::SignalListItemFactory { return; }; let (Some(row), Some(text)) = ( - item.child().and_downcast::(), + item.child().and_then(|child| EntryRow::of(&child)), item.item().and_downcast::(), ) else { return; }; let text = text.borrow::(); - row.set_title(&text.title); - row.set_subtitle(&text.subtitle); + row.title.set_label(&text.title); + row.subtitle.set_label(&text.subtitle); + // The row is labels only, so the item carries what a screen reader says. + item.set_accessible_label(&text.title); + item.set_accessible_description(&text.subtitle); }); factory } +// A list item's child is a plain widget: a GtkListBoxRow such as AdwActionRow belongs in a GtkListBox only. +struct EntryRow { + root: gtk::Box, + title: gtk::Label, + subtitle: gtk::Label, +} + +impl EntryRow { + fn new() -> Self { + // The file's names and hosts stay text: a GtkLabel reads markup only when told to. + let title = gtk::Label::builder() + .xalign(0.0) + .wrap(true) + .wrap_mode(gtk::pango::WrapMode::WordChar) + .build(); + let subtitle = gtk::Label::builder() + .xalign(0.0) + .wrap(true) + .wrap_mode(gtk::pango::WrapMode::WordChar) + .css_classes(["dim-label", "caption"]) + .build(); + let text = gtk::Box::builder() + .orientation(gtk::Orientation::Vertical) + .spacing(2) + .hexpand(true) + .valign(gtk::Align::Center) + .build(); + text.append(&title); + text.append(&subtitle); + // rich-list spaces the children of a row's box, so the labels sit one box further in. + let root = gtk::Box::new(gtk::Orientation::Horizontal, 0); + root.append(&text); + Self { root, title, subtitle } + } + + fn of(child: >k::Widget) -> Option { + let root = child.clone().downcast::().ok()?; + let text = root.first_child()?; + let title = text.first_child().and_downcast::()?; + let subtitle = title.next_sibling().and_downcast::()?; + Some(Self { root, title, subtitle }) + } +} + /// The SSH server the connection goes through, after any jump hosts. fn ssh_hop(ssh: &SavedSshConfig) -> String { let host = match ssh.port { @@ -319,6 +366,57 @@ mod tests { assert_eq!(lines(&checked), ["New · fake · app@db.internal:5432"]); } + #[gtk4::test] + fn the_rows_present_without_a_warning_and_name_each_connection() { + let root = tempfile::tempdir().expect("tempdir"); + let at = |dir: &str| { + tablepro_storage::StoragePaths::under(&root.path().join(dir), "tablepro", "app.tablepro.TablePro") + }; + let source = tablepro_storage::ConnectionStore::new(&at("source")); + source + .upsert_blocking(entry(ImportChange::Added, TlsConfig::disabled(), None).connection) + .expect("save"); + let exported = source.export_blocking().expect("export"); + let preview = tablepro_storage::ConnectionStore::new(&at("target")) + .preview_import_blocking(std::path::Path::new("/tmp/connections.json"), &exported) + .expect("preview"); + let window = adw::Window::builder().default_width(800).default_height(600).build(); + let dialog = import_preview_dialog(&preview, &drivers()); + + let previous = glib::log_set_always_fatal(glib::LogLevels::LEVEL_WARNING | glib::LogLevels::LEVEL_CRITICAL); + window.present(); + dialog.present(Some(&window)); + crate::test_support::after_next_frame(&dialog); + crate::test_support::drain_main_context(); + glib::log_set_always_fatal(previous); + + let list = crate::test_support::first_descendant_of_type::(&dialog).expect("the list"); + let item = list.first_child().expect("a row"); + let child = item.first_child().expect("the row's child"); + assert!(!child.is::(), "{}", child.type_().name()); + let labels: Vec = crate::test_support::descendants(&item) + .iter() + .filter_map(|widget| { + widget + .downcast_ref::() + .map(|label| label.label().to_string()) + }) + .collect(); + assert_eq!( + labels, + ["orders", "New · fake · app@db.internal:5432\nConnects without TLS"] + ); + if crate::test_support::test_backend_records_properties() { + assert!(gtk::test_accessible_has_property(&item, gtk::AccessibleProperty::Label)); + assert!(gtk::test_accessible_has_property( + &item, + gtk::AccessibleProperty::Description + )); + } + dialog.force_close(); + window.close(); + } + #[test] fn an_updated_entry_says_what_it_was_called() { let renamed = entry( diff --git a/linux/crates/app/src/ui/session_window/imp.rs b/linux/crates/app/src/ui/session_window/imp.rs index 72dc6df305..ad1835aba0 100644 --- a/linux/crates/app/src/ui/session_window/imp.rs +++ b/linux/crates/app/src/ui/session_window/imp.rs @@ -102,6 +102,9 @@ pub struct SessionWindow { /// The window opened for a file and has shown nothing else yet, so /// it closes again when another window already has that file. pub(super) opened_for_file: Cell, + // Read while no relational session was in front; the next one the + // window adopts opens them. + pub(super) waiting_scripts: RefCell>, /// Reads which filesystem a database file is on. `None` reads the /// real one; a test stands in for a network share. pub(super) classify_location: Cell>, diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index f56ba8b03c..f4b2aadba1 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -49,7 +49,7 @@ use libadwaita as adw; use libadwaita::prelude::AdwApplicationWindowExt; use tokio_util::sync::CancellationToken; -use tablepro_core::{DatabaseDriver, DriverError, SafeModeLevel, TransportClass}; +use tablepro_core::{DataModel, DatabaseDriver, DriverError, SafeModeLevel, TransportClass}; use tablepro_session::connect::StoreConnectionWrites; use tablepro_session::registry::FileIdentity; use tablepro_session::runtime::ProcessHold; @@ -169,6 +169,10 @@ impl SessionWindow { picker .banner() .connect_button_clicked(move |_| reset_input.deliver(SessionWindowInput::ResetConnectionList)); + let drop_input = input.clone(); + picker + .script_banner() + .connect_button_clicked(move |_| drop_input.deliver(SessionWindowInput::DropWaitingScripts)); let window = self.downgrade(); imp.list_observer.set(Some(list.connect_state_changed(move |_| { if let Some(window) = window.upgrade() { @@ -437,6 +441,12 @@ impl SessionWindow { SessionWindowInput::OpenSaved(saved) => self.on_open_saved(*saved), SessionWindowInput::OpenFile(file) => self.on_open_file(file), SessionWindowInput::FileResolved(resolved) => self.on_file_resolved(*resolved), + SessionWindowInput::OpenScript(file) => self.on_open_script(file), + SessionWindowInput::ScriptLoaded { file, text } => self.on_script_loaded(file, text), + SessionWindowInput::DropWaitingScripts => { + self.imp().waiting_scripts.borrow_mut().clear(); + self.show_waiting_scripts(); + } SessionWindowInput::Restore(key) => self.on_restore(key), SessionWindowInput::RestoreSaved { id, saved } => self.on_restore_saved(id, saved.map(|saved| *saved)), SessionWindowInput::RestoreUnread(key) => self.end_restore(&key), @@ -823,6 +833,9 @@ impl SessionWindow { self.render(); shell.add_session(&workspace); workspace.start(); + if workspace.data_model() == Some(DataModel::Relational) { + self.open_waiting_scripts(&workspace); + } self.sync_actions(); for warning in warnings { self.show_toast(&warning); @@ -875,7 +888,11 @@ impl SessionWindow { // A window that stayed to say the owner could not be // shown is the user's picker from then on. let opened_for_file = self.imp().opened_for_file.replace(false); - if self.present_owner(window_id, &key) && opened_for_file { + let presented = self.present_owner(window_id, &key); + if presented { + self.hand_waiting_scripts_to(window_id); + } + if presented && opened_for_file { self.close(); } None @@ -1077,6 +1094,10 @@ impl SessionWindow { } self.connect(ConnectSource::File(identity), CredentialDemand::default()); } + Ok(FileTarget::Script) => { + self.forget_unclaimed(&arrived); + self.on_open_script(gio::File::for_uri(&uri)); + } Err(error) => { tracing::warn!(%error, %uri, "could not open the file"); self.forget_unclaimed(&arrived); @@ -1098,6 +1119,142 @@ impl SessionWindow { } } + pub(crate) fn script_tab(&self, file: &gio::File) -> Option { + self.workspaces() + .iter() + .find_map(|workspace| workspace.editor_tab_for(file)) + } + + // Read the way Open SQL File reads, with the same limit and messages. + fn on_open_script(&self, file: gio::File) { + let Some(input) = self.input() else { + return; + }; + glib::spawn_future_local(async move { + let text = crate::ui::editor::load_text(&file).await; + input.deliver(SessionWindowInput::ScriptLoaded { file, text }); + }); + } + + // A script opens in the relational session in front and is never run. + fn on_script_loaded(&self, file: gio::File, text: Result) { + let text = match text { + Ok(text) => text, + Err(reason) => { + let name = file.parse_name().to_string(); + self.show_toast(&crate::i18n::gettext_f( + "{file} could not be opened. {reason}", + &[("file", &name), ("reason", &reason)], + )); + return; + } + }; + if let Some(tab) = self.script_tab(&file) { + self.on_focus_tab(tab); + return; + } + let front = self + .workspace() + .filter(|workspace| workspace.data_model() == Some(DataModel::Relational)) + .and_then(|workspace| workspace.sender()); + match front { + Some(input) => input.deliver(WorkspaceInput::OpenScript { file, text }), + None if self.page() != SessionPage::Shell => self.wait_for_session(file, text), + None => self.open_script_elsewhere(file, text), + } + } + + fn wait_for_session(&self, file: gio::File, text: String) { + { + let mut waiting = self.imp().waiting_scripts.borrow_mut(); + waiting.retain(|(kept, _)| !kept.equal(&file)); + waiting.push((file, text)); + } + self.show_waiting_scripts(); + } + + // A session that holds keys or documents runs no SQL, so the script + // asks for a connection in a window of its own. + fn open_script_elsewhere(&self, file: gio::File, text: String) { + let window = self + .application() + .and_downcast::() + .and_then(|app| app.open_window(WindowLaunch::Picker)) + .and_downcast::(); + match window { + Some(window) => window.send(SessionWindowInput::ScriptLoaded { file, text: Ok(text) }), + None => tracing::warn!("no window to open a SQL file in"), + } + } + + // The connection the user picked for them is in front there now. + fn hand_waiting_scripts_to(&self, window_id: u32) { + let Some(owner) = self + .application() + .and_then(|app| app.window_by_id(window_id)) + .and_downcast::() + else { + return; + }; + let waiting = self.imp().waiting_scripts.take(); + if waiting.is_empty() { + return; + } + for (file, text) in waiting { + owner.send(SessionWindowInput::ScriptLoaded { file, text: Ok(text) }); + } + self.show_waiting_scripts(); + } + + fn open_waiting_scripts(&self, workspace: &Workspace) { + let Some(input) = workspace.sender() else { + return; + }; + let waiting = self.imp().waiting_scripts.take(); + if waiting.is_empty() { + return; + } + for (file, text) in waiting { + input.deliver(WorkspaceInput::OpenScript { file, text }); + } + self.show_waiting_scripts(); + } + + fn show_waiting_scripts(&self) { + let Some(picker) = self.imp().picker.get() else { + return; + }; + let banner = picker.script_banner(); + let title = match self.imp().waiting_scripts.borrow().as_slice() { + [] => None, + [(file, _)] => Some(crate::i18n::gettext_f( + "Choose a connection to open “{name}”", + &[("name", &file_display_name(&file.uri()))], + )), + waiting => { + let count = waiting.len(); + Some(crate::i18n::ngettext_f( + "Choose a connection to open {n} SQL file", + "Choose a connection to open {n} SQL files", + u32::try_from(count).unwrap_or(u32::MAX), + &[("n", &count.to_string())], + )) + } + }; + match title { + Some(title) => { + banner.set_title(&title); + banner.set_revealed(true); + } + None => banner.set_revealed(false), + } + } + + #[cfg(test)] + pub(crate) fn script_banner(&self) -> Option { + self.imp().picker.get().map(PickerPage::script_banner) + } + /// The dialog may be editing a connection the window shows or already /// connects, or one open in another window: its edits are kept for /// that connection's next connect. diff --git a/linux/crates/app/src/ui/session_window/picker_page.rs b/linux/crates/app/src/ui/session_window/picker_page.rs index 6c50c506ca..5bd9c5f01b 100644 --- a/linux/crates/app/src/ui/session_window/picker_page.rs +++ b/linux/crates/app/src/ui/session_window/picker_page.rs @@ -26,6 +26,8 @@ mod imp { #[template_child] pub connection_list_banner: TemplateChild, #[template_child] + pub script_banner: TemplateChild, + #[template_child] pub welcome_slot: TemplateChild, } @@ -93,9 +95,10 @@ mod imp { } glib::wrapper! { - /// The saved connections, with a header bar of their own and the - /// banner that says when the list could not be read. It sits outside - /// any split view, so no empty sidebar can show while disconnected. + /// The saved connections, with a header bar of their own, the banner + /// that says when the list could not be read and the one that asks + /// for a connection to open SQL files in. It sits outside any split + /// view, so no empty sidebar can show while disconnected. pub struct PickerPage(ObjectSubclass) @extends adw::Bin, gtk::Widget, @implements gtk::Accessible, gtk::Buildable, gtk::ConstraintTarget; @@ -122,6 +125,10 @@ impl PickerPage { self.imp().connection_list_banner.get() } + pub fn script_banner(&self) -> adw::Banner { + self.imp().script_banner.get() + } + pub fn search_entry(&self) -> gtk::SearchEntry { self.imp().search_entry.get() } diff --git a/linux/crates/app/src/ui/session_window/session_window_input.rs b/linux/crates/app/src/ui/session_window/session_window_input.rs index 0e23a1d9e9..a102577c6a 100644 --- a/linux/crates/app/src/ui/session_window/session_window_input.rs +++ b/linux/crates/app/src/ui/session_window/session_window_input.rs @@ -26,6 +26,14 @@ pub(crate) enum SessionWindowInput { OpenFile(gio::File), /// The file resolved, off the GTK thread, to the session it opens. FileResolved(Box), + /// Open a SQL script in an editor tab, from Files or the command line. + OpenScript(gio::File), + ScriptLoaded { + file: gio::File, + text: Result, + }, + /// The waiting scripts banner's Cancel. + DropWaitingScripts, /// Open one of the sessions the window was opened to restore. Restore(SessionKey), /// The saved connections could not be read for this restored key, diff --git a/linux/crates/app/src/ui/session_window/tests/open_file.rs b/linux/crates/app/src/ui/session_window/tests/open_file.rs index 1ce469a1ef..b58cfdefe5 100644 --- a/linux/crates/app/src/ui/session_window/tests/open_file.rs +++ b/linux/crates/app/src/ui/session_window/tests/open_file.rs @@ -1,11 +1,15 @@ //! Files the application hands to its windows, from Open With or the //! command line. +use std::time::Duration; + use gtk4::prelude::*; +use tablepro_session::fakes::FakeEstablish; +use tablepro_test_support::{CallCounters, ConnectionScript}; use super::super::*; -use super::{TestWindow, WAIT}; -use crate::test_support::wait_until; +use super::{Script, TestWindow, WAIT, editors}; +use crate::test_support::{drain_main_context, wait_until}; /// An empty file, which SQLite reads as an empty database. fn database(dir: &tempfile::TempDir, name: &str) -> gio::File { @@ -106,3 +110,199 @@ fn a_window_resolving_a_file_takes_no_other() { test.window.send(SessionWindowInput::OpenConnectDialog); wait_until(WAIT, || !test.window.is_idle()).expect("the connect dialog to count as busy"); } + +fn script(test: &TestWindow, name: &str, text: &str) -> gio::File { + let path = test.dir().join(name); + crate::test_support::write_text_file(&path, text); + gio::File::for_path(path) +} + +// The test's own session would be restored from the record in a second +// window, which an open before any activate reads. +fn open(test: &TestWindow, files: &[gio::File]) { + test.services + .settings + .set_restore_session(false) + .expect("turn restoring off"); + test.app.open(files, ""); +} + +fn text_of(editor: &crate::ui::editor::SqlEditor) -> String { + let buffer = editor.buffer(); + buffer.text(&buffer.start_iter(), &buffer.end_iter(), false).to_string() +} + +fn bound_to(editor: &crate::ui::editor::SqlEditor, file: &gio::File) -> bool { + editor.file().is_some_and(|bound| bound.equal(file)) +} + +fn script_editor(window: &SessionWindow, file: &gio::File) -> crate::ui::editor::SqlEditor { + let mut found = None; + wait_until(WAIT, || { + found = editors(window).into_iter().find(|editor| bound_to(editor, file)); + found.is_some() + }) + .expect("an editor tab bound to the file"); + found.expect("the editor") +} + +fn script_banner(test: &TestWindow) -> adw::Banner { + test.window.script_banner().expect("the picker's script banner") +} + +#[gtk4::test] +fn a_sql_file_opens_in_the_session_in_front_without_running() { + let test = TestWindow::connected("OpenScript"); + let counters = test.connection().counters(); + let file = script(&test, "report.sql", "DELETE FROM orders;\n"); + + open(&test, std::slice::from_ref(&file)); + let editor = script_editor(&test.window, &file); + drain_main_context(); + + assert_eq!(text_of(&editor), "DELETE FROM orders;\n"); + assert!(!editor.has_unsaved_file_changes(), "the tab differs from its file"); + let page = test + .window + .shell() + .and_then(|shell| shell.tab_view().selected_page()) + .expect("a tab in front"); + assert_eq!(page.title(), "report.sql"); + assert!(!editor.is_running(), "the script ran"); + assert_eq!(CallCounters::get(&counters.open_editor_session), 0, "the script ran"); + assert_eq!(test.connector.calls().len(), 1, "the script was opened as a database"); + assert_eq!(test.app.windows().len(), 1, "the script opened another window"); +} + +#[gtk4::test] +fn a_sql_file_on_the_picker_waits_for_a_connection() { + let test = TestWindow::picker("ScriptWaits"); + let banner = script_banner(&test); + let file = script(&test, "report.sql", "SELECT 1;"); + + open(&test, std::slice::from_ref(&file)); + wait_until(WAIT, || banner.is_revealed()).expect("the banner asking for a connection"); + assert_eq!(banner.title(), "Choose a connection to open “report.sql”"); + assert!(test.connector.calls().is_empty(), "the script was opened as a database"); + assert_eq!(test.window.page_name(), "picker"); + + test.window + .send(SessionWindowInput::OpenSaved(Box::new(test.saved.clone()))); + test.wait_for_page("workspace"); + let editor = script_editor(&test.window, &file); + + assert_eq!(text_of(&editor), "SELECT 1;"); + assert!(!banner.is_revealed(), "the banner stayed once the script opened"); + assert_eq!(test.app.windows().len(), 1); +} + +#[gtk4::test] +fn the_banner_cancel_drops_the_waiting_scripts() { + let test = TestWindow::picker("ScriptCancel"); + let banner = script_banner(&test); + let files = [ + script(&test, "first.sql", "SELECT 1;"), + script(&test, "second.sql", "SELECT 2;"), + ]; + + open(&test, &files); + wait_until(WAIT, || banner.title() == "Choose a connection to open 2 SQL files").expect("both scripts waiting"); + assert!(banner.is_revealed()); + banner.emit_by_name::<()>("button-clicked", &[]); + wait_until(WAIT, || !banner.is_revealed()).expect("the banner to go"); + + test.window + .send(SessionWindowInput::OpenSaved(Box::new(test.saved.clone()))); + test.wait_for_page("workspace"); + let _ = wait_until(Duration::from_millis(300), || false); + drain_main_context(); + + assert!( + editors(&test.window) + .iter() + .all(|editor| files.iter().all(|file| !bound_to(editor, file))), + "a dropped script opened" + ); +} + +#[gtk4::test] +fn a_sql_file_while_connecting_opens_once_the_session_joins() { + let test = TestWindow::with_script( + "ScriptConnecting", + Script { + establish: vec![FakeEstablish::Late { + after: Duration::from_secs(2), + script: ConnectionScript::default(), + }], + ..Script::default() + }, + ); + test.window + .send(SessionWindowInput::OpenSaved(Box::new(test.saved.clone()))); + test.wait_for_page("connecting"); + let file = script(&test, "report.sql", "SELECT 1;"); + + open(&test, std::slice::from_ref(&file)); + wait_until(WAIT, || !test.window.imp().waiting_scripts.borrow().is_empty()) + .expect("the script to wait for the connect"); + assert_eq!(test.window.page_name(), "connecting"); + let editor = script_editor(&test.window, &file); + + assert_eq!(test.window.page_name(), "workspace"); + assert_eq!(text_of(&editor), "SELECT 1;"); + assert!(test.window.imp().waiting_scripts.borrow().is_empty()); + assert_eq!(test.connector.calls().len(), 1, "the script was opened as a database"); +} + +#[gtk4::test] +fn an_unreadable_sql_file_says_why_and_opens_nothing() { + let test = TestWindow::connected("ScriptNotUtf8"); + let path = test.dir().join("latin1.sql"); + std::fs::write(&path, b"SELECT 'caf\xe9';").expect("seed the script"); + let file = gio::File::for_path(&path); + + open(&test, std::slice::from_ref(&file)); + wait_until(WAIT, || { + test.window + .imp() + .toasts + .borrow() + .iter() + .any(|toast| toast.title().contains("not UTF-8 text")) + }) + .expect("a toast saying why"); + drain_main_context(); + + assert!( + editors(&test.window).iter().all(|editor| !bound_to(editor, &file)), + "a tab opened on the file" + ); + assert!(!script_banner(&test).is_revealed(), "the window asked for a connection"); +} + +#[gtk4::test] +fn a_sql_file_already_open_brings_its_tab_forward() { + let test = TestWindow::connected("ScriptTwice"); + let file = script(&test, "report.sql", "SELECT 1;"); + open(&test, std::slice::from_ref(&file)); + let editor = script_editor(&test.window, &file); + let shell = test.window.shell().expect("the shell"); + let in_front = || { + shell + .tab_view() + .selected_page() + .is_some_and(|page| page.child() == editor.clone().upcast::()) + }; + WidgetExt::activate_action(&test.window, "win.new-editor-tab", None).expect("win.new-editor-tab"); + wait_until(WAIT, || !in_front()).expect("another tab in front"); + + test.app.open(std::slice::from_ref(&file), ""); + wait_until(WAIT, || in_front()).expect("the script's tab in front"); + + let bound = editors(&test.window) + .iter() + .filter(|editor| bound_to(editor, &file)) + .count(); + assert_eq!(bound, 1, "a second tab opened on the file"); + assert_eq!(test.app.windows().len(), 1); +} diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index d2d5419edb..c37b76bc1d 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -180,6 +180,8 @@ pub(crate) struct WorkspaceModel { driver_name: String, /// Writes the tab list a moment after the last change. workspace: Rc, + // Until the saved tabs are back, a script waits, so a restored tab bound to its file can take it. + loading_scripts: Option>, _watches: SessionWatches, /// Tells the user once when editor drafts start failing to land. _draft_failures: tablepro_session::runtime::WatchForwarder, @@ -616,6 +618,11 @@ pub(crate) enum WorkspaceInput { sql: String, database: Option, }, + /// A SQL file handed to TablePro, in a new editor tab bound to it. + OpenScript { + file: gio::File, + text: String, + }, DropTablePrompt { schema: Option, table: String, @@ -858,6 +865,7 @@ impl Workspace { saved_queries_dialog: None, import_dialog: None, workspace: persistence, + loading_scripts: Some(Vec::new()), _watches: watches, _draft_failures: draft_failures, }; @@ -905,6 +913,12 @@ impl Workspace { self.imp().data_model.get().copied() } + /// The editor tab bound to `file`. `None` while the workspace is + /// handling input. + pub(crate) fn editor_tab_for(&self, file: &gio::File) -> Option { + self.read(|model| model.editor_tab_for(file)).flatten() + } + /// The tab `page` shows, when it is one of this session's. Read from /// the index, so it answers while the workspace handles its input. pub(crate) fn tab_id(&self, page: &adw::TabPage) -> Option { @@ -1288,6 +1302,7 @@ impl WorkspaceModel { WorkspaceInput::EditStructureTab { schema, table } => self.on_edit_structure_tab(schema, table), WorkspaceInput::ShowCreateTableForExisting { schema, table } => self.on_show_create_table(schema, table), WorkspaceInput::OpenInEditor { sql, database } => self.open_sql_in_editor(sql, database), + WorkspaceInput::OpenScript { file, text } => self.open_script(&file, text), WorkspaceInput::DropTablePrompt { schema, table } => self.on_drop_table_prompt(schema, table), WorkspaceInput::DropTableChecked { table, referencing } => self.on_drop_table_checked(table, &referencing), WorkspaceInput::DropTableConfirmed { schema, table } => self.on_drop_table_confirmed(schema, table), diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index ef22586280..86fa361741 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -88,6 +88,9 @@ impl WorkspaceModel { self.workspace .attach(key, &self.tab_view, self.workspace_tabs.clone(), self.index.clone()); self.persist_workspace_state(); + for (file, text) in self.loading_scripts.take().unwrap_or_default() { + self.open_script(&file, text); + } } /// Delete the session's drafts that neither the saved tabs nor an @@ -208,6 +211,33 @@ impl WorkspaceModel { }); } + pub(super) fn open_script(&mut self, file: &gio::File, text: String) { + if let Some(waiting) = self.loading_scripts.as_mut() { + waiting.push((file.clone(), text)); + return; + } + if let Some(id) = self.editor_tab_for(file) { + self.focus_tab(id); + return; + } + self.open_editor(TabSpec::Editor { + initial_query: Some(text), + draft: None, + file: Some(EditorFileRecord { + uri: file.uri().to_string(), + modified: false, + }), + database: None, + }); + } + + pub(super) fn editor_tab_for(&self, file: &gio::File) -> Option { + self.workspace_tabs.borrow().iter().find_map(|(id, tab)| match tab { + WorkspaceTab::Editor(slot) if slot.editor.file().is_some_and(|bound| bound.equal(file)) => Some(*id), + _ => None, + }) + } + // AdwTabView moves the focus to the page it selects only from the page // that held it, so an editor opened from the sidebar, a header button // or a key pressed there would leave the typing where it was. diff --git a/linux/crates/app/src/workspace/workspace_store.rs b/linux/crates/app/src/workspace/workspace_store.rs index dd8d0c99c0..a4f8a5a81b 100644 --- a/linux/crates/app/src/workspace/workspace_store.rs +++ b/linux/crates/app/src/workspace/workspace_store.rs @@ -177,8 +177,9 @@ impl WorkspaceStore { } /// Delete everything kept for a session whose connection is gone: - /// its saved tabs, any copy of them Reset moved aside, and its - /// drafts. A record still queued for it is never written. + /// its saved tabs, any copy of them Reset moved aside or an upgrade + /// kept, and its drafts. A record still queued for it is never + /// written. /// /// It goes through the writer, so a write queued before it cannot /// land after it and bring the file back. It also waits for the @@ -421,6 +422,7 @@ fn purge_blocking(path: &Path, drafts: &DraftStore, scope: &DraftScope) -> Resul _ => Ok(()), }; let copies = tablepro_storage::fs::remove_moved_aside_blocking(path) + .and_then(|_| tablepro_storage::upgrade_copies::remove_all_blocking(path)) .map(drop) .map_err(|error| error.to_string()); let drafts = drafts.delete_scope_blocking(scope).map_err(|error| error.to_string()); @@ -428,8 +430,8 @@ fn purge_blocking(path: &Path, drafts: &DraftStore, scope: &DraftScope) -> Resul } /// Every session named with `prefix` in `dir` or in the drafts, whether -/// it left a file, only a copy moved aside as `.json.corrupt-…`, -/// or only drafts. Each is purged as `purge_blocking` does; the first +/// it left a file, only a copy moved aside as `.json.corrupt-…` or +/// kept before an upgrade as `.json.v…`, or only drafts. Each is purged as `purge_blocking` does; the first /// failure is the one reported. fn purge_named_blocking(dir: &Path, drafts: &DraftStore, prefix: &str) -> Result<(), WorkspaceSaveError> { let mut stems = std::collections::BTreeSet::new(); @@ -470,6 +472,8 @@ async fn write_record( let outcome = tasks .spawn_blocking_task(move || { let bytes = encode_document(&record).map_err(|error| WorkspaceSaveError::Write(error.to_string()))?; + tablepro_storage::upgrade_copies::keep_before_write_blocking::(&path) + .map_err(|error| WorkspaceSaveError::Write(error.to_string()))?; write(&path, &bytes).map_err(|error| WorkspaceSaveError::Write(error.to_string())) }) .await @@ -887,6 +891,51 @@ mod tests { assert_eq!(fixture.on_disk(&other), record(2)); } + #[derive(serde::Serialize, serde::Deserialize)] + struct Bumped { + tabs: Vec, + } + + fn same_shape(document: serde_json::Value) -> Result { + Ok(document) + } + + // A format bumped once, so the writer's copy is tested without any format the app shipped. + impl VersionedDocument for Bumped { + const KIND: &'static str = "bumped"; + const VERSION: u32 = 2; + const UPGRADES: &'static [tablepro_storage::document::Upgrade] = &[same_shape]; + } + + #[tokio::test] + async fn an_older_file_is_copied_aside_before_its_first_write_and_purged_with_the_session() { + let fixture = Fixture::new(); + let key = key(); + let older = br#"{"version": 1, "tabs": ["orders"]}"#; + let path = fixture.seed(&key, older); + let write: Arc = Arc::new(tablepro_storage::fs::write_private_blocking); + let copies = || tablepro_storage::upgrade_copies::copies_blocking(&path).expect("copies"); + + for tabs in [vec!["orders".to_owned()], vec!["orders".to_owned(), "items".to_owned()]] { + write_record(&paused_tasks(), &write, path.clone(), Bumped { tabs }) + .await + .expect("written"); + } + + let kept = copies(); + assert_eq!(kept.len(), 1, "{kept:?}"); + assert_eq!(std::fs::read(&kept[0].path).expect("read"), older); + let written = std::fs::read_to_string(&path).expect("read"); + assert!( + written.contains("\"version\": 2") && written.contains("items"), + "{written}" + ); + + fixture.store.purge(&key).await.expect("purged"); + + assert!(copies().is_empty(), "a copy kept before an upgrade outlived the purge"); + } + /// A deleted connection's other databases leave nothing behind, /// whether one left its saved tabs, only a copy Reset moved aside, or /// only drafts. diff --git a/linux/crates/app/tests/desktop_entry.rs b/linux/crates/app/tests/desktop_entry.rs index ca052de5be..890fe0f78e 100644 --- a/linux/crates/app/tests/desktop_entry.rs +++ b/linux/crates/app/tests/desktop_entry.rs @@ -1,5 +1,5 @@ -//! The desktop entry offers TablePro for SQLite files and hands them -//! over the way the application expects. +//! The desktop entry offers TablePro for SQLite files and SQL scripts +//! and hands them over the way the application expects. use std::path::Path; @@ -13,6 +13,12 @@ fn entry() -> Result { Ok(entry) } +fn metainfo() -> std::io::Result { + std::fs::read_to_string( + Path::new(env!("CARGO_MANIFEST_DIR")).join("../../data/app.tablepro.TablePro.metainfo.xml.in.in"), + ) +} + fn list(entry: &glib::KeyFile, group: &str, key: &str) -> Result, glib::Error> { Ok(entry .string_list(group, key)? @@ -33,6 +39,43 @@ fn sqlite_files_are_offered_to_tablepro() { ); } +#[test] +fn sql_scripts_are_offered_to_tablepro() { + let entry = entry().expect("the desktop entry parses"); + + let types = list(&entry, MAIN, "MimeType").expect("a MimeType list"); + + assert!( + types.contains(&"application/sql".to_owned()), + "the entry does not take SQL scripts: {types:?}" + ); +} + +// Software centres read the media types from the metainfo, not the entry. +#[test] +fn the_metainfo_provides_every_type_the_entry_takes() { + let entry = entry().expect("the desktop entry parses"); + let xml = metainfo().expect("the metainfo reads"); + + let mut taken = list(&entry, MAIN, "MimeType").expect("a MimeType list"); + let provides = xml + .split_once("") + .and_then(|(_, rest)| rest.split_once("")) + .map(|(provides, _)| provides) + .expect("a provides element"); + let mut provided: Vec = provides + .split("") + .skip(1) + .filter_map(|rest| rest.split_once("")) + .map(|(media_type, _)| media_type.trim().to_owned()) + .collect(); + taken.retain(|media_type| !media_type.is_empty()); + taken.sort(); + provided.sort(); + + assert_eq!(provided, taken); +} + #[test] fn files_reach_the_running_application() { let entry = entry().expect("the desktop entry parses"); diff --git a/linux/data/app.tablepro.TablePro.desktop.in.in b/linux/data/app.tablepro.TablePro.desktop.in.in index 7b568f3d18..9420b75260 100644 --- a/linux/data/app.tablepro.TablePro.desktop.in.in +++ b/linux/data/app.tablepro.TablePro.desktop.in.in @@ -9,7 +9,7 @@ Icon=@app-id@ Terminal=false Type=Application Categories=Development;Database; -MimeType=application/vnd.sqlite3; +MimeType=application/vnd.sqlite3;application/sql; DBusActivatable=true StartupNotify=true X-GNOME-UsesNotifications=true diff --git a/linux/data/app.tablepro.TablePro.metainfo.xml.in.in b/linux/data/app.tablepro.TablePro.metainfo.xml.in.in index fbe5359ec0..157e21fed4 100644 --- a/linux/data/app.tablepro.TablePro.metainfo.xml.in.in +++ b/linux/data/app.tablepro.TablePro.metainfo.xml.in.in @@ -65,6 +65,8 @@ tablepro + application/vnd.sqlite3 + application/sql diff --git a/linux/data/resources/ui/picker-page.ui b/linux/data/resources/ui/picker-page.ui index 07d824c858..b613f4b774 100644 --- a/linux/data/resources/ui/picker-page.ui +++ b/linux/data/resources/ui/picker-page.ui @@ -52,6 +52,12 @@ false + + + false + Cancel + + diff --git a/linux/data/tablepro.1 b/linux/data/tablepro.1 index a217886f83..1d650a3a65 100644 --- a/linux/data/tablepro.1 +++ b/linux/data/tablepro.1 @@ -13,7 +13,9 @@ and reaches servers behind an SSH bastion through the system OpenSSH client. .PP Each .I FILE -is a SQLite or DuckDB database, opened in its own window. +is a SQLite or DuckDB database, opened in its own window, +or a SQL file, opened in an editor tab of the connection in front. +A SQL file never runs by itself; with no connection open, the window asks for one. A running TablePro opens the files itself and the new launch exits. .SH OPTIONS .TP diff --git a/linux/docs/releasing.md b/linux/docs/releasing.md index fcfe44efdc..6ef48fc0f1 100644 --- a/linux/docs/releasing.md +++ b/linux/docs/releasing.md @@ -59,4 +59,4 @@ The next version's release commit sets `Release` back to 1. ## Downgrades -An older build never writes over what a newer one stored; it refuses it until the user resets it or upgrades again. [troubleshooting.md](troubleshooting.md#after-a-downgrade) tells users what they see. +A build refuses a file in a storage format newer than its own and never writes over it; a file in a format it knows it reads and writes, whichever release wrote it. Before the first write in a newer format, a build keeps a copy of the older file beside it ([storage.md](storage.md#copies-before-a-format-upgrade)). [troubleshooting.md](troubleshooting.md#after-a-downgrade) tells users what they see and how to put a copy back. diff --git a/linux/docs/state-management.md b/linux/docs/state-management.md index 1e3642c33d..4ef333a6a3 100644 --- a/linux/docs/state-management.md +++ b/linux/docs/state-management.md @@ -118,7 +118,7 @@ A finished run goes to `HistoryService::record` with the text that was run, `DEL ### The tab's file -Open SQL File (Ctrl+O) and Save As (Ctrl+Shift+S) bind the tab to a `.sql` file, through the file chooser portal; Save (Ctrl+S) writes to it, or asks for one. The tab is named after the file, and the buffer's own modified flag says whether its text differs from the file's, so Undo back to the saved text clears it. The tab's record keeps the file's URI and that flag (`EditorFileRecord`), and its draft keeps the text, so a restore brings back both. +Open SQL File (Ctrl+O) and Save As (Ctrl+Shift+S) bind the tab to a `.sql` file, through the file chooser portal; Save (Ctrl+S) writes to it, or asks for one. A `.sql` file handed over from Files or the command line opens in a new tab bound to it (`WorkspaceInput::OpenScript`). The tab is named after the file, and the buffer's own modified flag says whether its text differs from the file's, so Undo back to the saved text clears it. The tab's record keeps the file's URI and that flag (`EditorFileRecord`), and its draft keeps the text, so a restore brings back both. Closing the tab asks about unsaved file changes through the same Save Changes prompt as row and structure edits (`DirtyKind::UnsavedFile`), and its Save writes the file. Closing the window does not ask: the draft keeps the text for the next session. diff --git a/linux/docs/storage.md b/linux/docs/storage.md index ecc8a25fab..6793ad82c2 100644 --- a/linux/docs/storage.md +++ b/linux/docs/storage.md @@ -18,9 +18,10 @@ The app's data goes to the store its shape calls for. The core's `storage` crate `ConnectionStore` holds the saved-connection list, one store per storage root: - `load_blocking`, `list_blocking`, `upsert_blocking`, `insert_after_blocking`, `update_blocking`, `remove_blocking`, `touch_last_opened_blocking`, the group and favourite operations below, `preview_import_blocking`, `apply_import_blocking` and `export_blocking` run off the GTK thread. A change holds the store's lock across read, change and write, so two writers never lose an entry. `update_blocking` changes one entry as the list holds it at the write, so a last-opened stamp from another window in between is kept. -- A write goes through `write_private_blocking` first and the cached list changes only once it landed, so a failed write leaves both as they were. +- A write goes through `write_document_blocking` first and the cached list changes only once it landed, so a failed write leaves both as they were. The first write over a file an older build wrote keeps a copy of it first ([Copies before a format upgrade](#copies-before-a-format-upgrade)). +- A file that another version wrote in a newer format is refused; one in the same format is read and written whichever build wrote it. - `subscribe` hands out a watch of `ConnectionListSnapshot`, so every picker sees every writer's change. -- A file that cannot be read, or that another version wrote, leaves the store `Unavailable`, and it refuses every write until the user picks Reset (`reset_unreadable_blocking`), which moves the file aside. +- A file that cannot be read, or that holds a format newer than this build's, leaves the store `Unavailable`, and it refuses every write until the user picks Reset (`reset_unreadable_blocking`), which moves the file aside. - A `SavedConnection` does **not** carry a password. Secrets live in the keyring, keyed by the connection's UUID, so a rename or a new host keeps them. - `safe_mode` is the connection's Safe Mode level: `off`, `confirm_writes` or `read_only`. It has no default, since guessing Off would drop a guard. Version 1 of the file had a `read_only` switch instead, which reads as `read_only` when on and `off` when off. - `external_access` is what MCP clients may do on the connection: `blocked`, `read_only` or `read_write` ([ADR 0022](decisions/0022-ai-assistant-and-mcp-server.md)). A file written before it existed reads as `read_only`, the level a new connection starts at. @@ -105,8 +106,9 @@ leaves nothing unreachable behind when a step fails: secret with no entry is orphaned in the keyring. 2. The entry in `connections.json`. 3. Everything kept for it: column layouts, filters, the saved tabs with any - copy Reset moved aside, the editor drafts (`WorkspaceStore::purge`), the - saved queries, and the query history when the user turned that on. A + copy Reset moved aside or an upgrade kept, the editor drafts + (`WorkspaceStore::purge`), the saved queries, and the query history when + the user turned that on. A failure here is reported and does not bring the entry back. A connection open or connecting in a window is not deleted; the dialog @@ -160,6 +162,10 @@ Everything private goes through `tablepro_storage::fs`: and the new file is 0600 whatever mode the old one had. - `append_private_blocking` appends to a file it creates 0600 when absent, for the crash report, which a second panic adds to. +- `document::write_document_blocking` encodes a versioned document and + writes it with `write_private_blocking`, after keeping a copy of a file + an older build wrote ([Copies before a format upgrade](#copies-before-a-format-upgrade)). + Every store writes its documents through it. - `create_private_file_if_missing` pre-creates an empty 0600 file. SQLite creates its database, WAL and SHM with the umask mode, so pre-creating the database is what keeps all three off world-readable. @@ -190,11 +196,15 @@ at compile time with `sqlx::migrate!` and run by `QueryHistory::open`: file with its own fixture test. Editing one in place changes its checksum, which makes sqlx refuse every database that already applied it. -A database carrying a version this build has no migration for is refused -with `StorageError::HistoryNewerThanApp` and nothing is written, so an -older binary cannot damage a newer file. Any other migration failure is +A database carrying a migration this build does not have is refused with +`StorageError::HistoryNewerThanApp` and nothing is written, so a build never +changes a schema newer than its own. A database at the same schema is read +and written by any build. Any other migration failure is `StorageError::HistoryMigration`. +Before a migration runs on a database that already has some, `open` keeps a +copy of it ([Copies before a format upgrade](#copies-before-a-format-upgrade)). + `QueryHistory::record` refuses a query over 1 MiB with `StorageError::TooLarge`. @@ -241,6 +251,8 @@ can tell whether a re-pinned entry falls in the pages already read. - `delete_for_connection` removes one connection's entries, pinned ones included, for a connection deleted along with its history, then runs `optimize` as a prune does. +- Both delete the same rows from every copy an upgrade kept, then `VACUUM` + it, so nothing of them stays on disk. A copy keeps its saved queries. - `connections_with_history` lists each connection the history holds under the name of its latest run, so a renamed connection shows once. @@ -363,8 +375,36 @@ A keyring failure is a `SecretError` from `tablepro-core`, not a `StorageError`; ## Changing a format - A JSON document is a `VersionedDocument` with a `version` field. A new shape bumps its `VERSION` and appends a step to its `UPGRADES`, which turns a file of the version before into the new shape. `decode_document` runs an older file through every step it is behind, oldest first, so a user who skipped releases keeps their data. A step that shipped is never changed or dropped. -- The upgrade happens in memory. The file stays as it was until the next save writes the current version. -- A file older than the first step, one a step cannot read, or one from a newer build is never written over. The connection list and a workspace wait for the user's Reset, which moves the file aside; the smaller state files move an unusable file aside themselves. +- The upgrade happens in memory. The file stays as it was until the next save, which keeps a copy of it and then writes the current version ([Copies before a format upgrade](#copies-before-a-format-upgrade)). +- A file older than the first step, one a step cannot read, or one in a format newer than the build's is never written over. The connection list and a workspace wait for the user's Reset, which moves the file aside; the smaller state files move an unusable file aside themselves. - Each released version of the connection list and the workspace file has a golden file that a test reads into the model it was written from: `core/crates/storage/tests/fixtures/connections-v1.json`, `connections-v2.json`, `connections-v3.json`, and `crates/app/tests/fixtures/workspace-v1.json` and `workspace-v2.json`. A committed fixture is never edited. A format change adds the new version's fixture and updates the expected models of the old ones, which now read through the upgrade. - The history database moves forward by sqlx migration: a new timestamped file under `core/crates/storage/migrations/` with its own fixture test, never an edit to an applied one. - Update this page, and add a fragment to the Linux changelog. + +## Copies before a format upgrade + +A file an older build wrote is copied beside itself before the first write +that replaces it with a newer format ([core ADR 0011](../../core/docs/decisions/0011-copies-before-a-format-upgrade.md)): + +- The copy is `.v-`, with `-` after it for a + second copy in the same second, such as `connections.json.v1-1762300000`. + `` is the format the copy + holds, the document's `version` or the history's newest migration. +- `tablepro_storage::upgrade_copies` makes them. A JSON document's copy is its + bytes as read, written 0600 by `write_private_blocking`; a copy whose bytes + match the newest one is not made again. The history's is `VACUUM INTO` through + the open connection, so it holds what the WAL holds too, into a 0600 + `.partial` file that is synced and renamed into place. +- Each file keeps its newest 3 copies (`KEPT_PER_FILE`); a new one deletes the + oldest past that. +- A copy of a session's tabs names their editor drafts and holds none of their + text. Drafts have no format version, so the older build reads them as they + are now; a tab closed since the copy comes back without its text. +- `count_for_connection` counts the history's copies too, so the queries of a + deleted connection that only a copy still holds can go with it. +- A copy that cannot be made fails the write or the open, and the file is + left as it was. The error names the copy's path. +- Reading never makes a copy. Nothing restores one on its own: + [troubleshooting.md](troubleshooting.md#after-a-downgrade) has the steps. +- `upgrade_copies` tests the hook with a synthetic version bump of a document + and of a migration list, so it needs no format the app shipped. diff --git a/linux/docs/troubleshooting.md b/linux/docs/troubleshooting.md index ff441f1272..4361637c31 100644 --- a/linux/docs/troubleshooting.md +++ b/linux/docs/troubleshooting.md @@ -83,13 +83,46 @@ Open an issue at https://github.com/TableProApp/TablePro/issues with: ## After a downgrade -An older version never changes a file a newer one wrote. It refuses the file instead, until the newer version is installed again or you reset it: +Each file TablePro keeps carries the version of its format. A build refuses a file in a format newer than it knows, and never writes over it. A format does not change with every release, so an older release still reads and changes a file a newer one wrote in the same format. The refusals look like this: - The connection list shows "Saved connections were created by a newer version of TablePro". Reset… moves the file aside, under a name the toast shows, and starts an empty list. - A connection's saved tabs do not reopen, and a toast says "Saved tabs could not be read" with Reset. -- The query history says it could not be read. +- The query history and saved queries say "The query history was written by a newer version of TablePro". +- Column widths and filters saved in the newer format are left as they are, and the older release does not save its own. -Installing the newer version again brings all of it back, unless it was reset. Column widths and filters saved by the newer version are left as they are, and the older version does not save its own. +Installing the newer release again brings all of it back, unless it was reset. + +### Going back to what the older release saved + +Before a release writes a file in a newer format for the first time, it keeps a copy of the file as it was, in the same folder, named after the file, the format version it holds and the time in seconds, such as `connections.json.v1-1762300000`. It keeps the newest three copies of each file. Clear History, and deleting a connection with its history, delete that history from the copies too. + +To give the older release its files back: + +1. Quit TablePro. A second launch hands over to the running one, so check that no `tablepro` process is left. +2. List the copies, newest last, in the folders from [Where TablePro keeps its files](#where-tablepro-keeps-its-files): + + ```sh + ls -ltr ~/.config/tablepro/ ~/.local/state/tablepro/ ~/.local/state/tablepro/workspaces/ + ``` + +3. Move the current file aside rather than deleting it. It holds what the newer release saved, and the newer release reads it again. For the history, move its `-wal` and `-shm` files with it, if they are there: + + ```sh + cd ~/.local/state/tablepro + mv history.db history.db.newer + mv history.db-wal history.db-wal.newer + mv history.db-shm history.db-shm.newer + ``` + +4. Copy the newest copy back under the file's own name. For the connection list: + + ```sh + cp ~/.config/tablepro/connections.json.v1-1762300000 ~/.config/tablepro/connections.json + ``` + +5. Start the older release. + +What was saved after the copy was made, such as a connection added in the newer release, is only in the file you moved aside. The Flatpak keeps the same files under `~/.var/app/app.tablepro.TablePro/`, in `config/tablepro/` and `.local/state/tablepro/`. ## Where TablePro keeps its files @@ -101,5 +134,6 @@ Installing the newer version again brings all of it back, unless it was reset. C | `~/.local/state/tablepro/workspaces/` | Each connection's tabs, and the connections open at the last quit | | `~/.local/share/tablepro/drafts/` | The text of each SQL editor tab | | `~/.local/state/tablepro/crashes/` | Crash reports | +| `.v-` beside a file above | A copy kept before an upgrade changed the file's format ([After a downgrade](#after-a-downgrade)) | Preferences are in GSettings: `gsettings list-recursively app.tablepro.TablePro` lists them. `man tablepro` has the same list. diff --git a/linux/po/README.md b/linux/po/README.md index 7a3d1d2a19..07600387be 100644 --- a/linux/po/README.md +++ b/linux/po/README.md @@ -52,8 +52,10 @@ Start from the template CI builds, so you do not need gettext 1.0: msginit --locale=xx --input=po/tablepro.pot --output=po/xx.po ``` -3. Add the code on its own line to [`LINGUAS`](LINGUAS) once the - translation is complete enough to ship. From then on: +3. Add the code on its own line to [`LINGUAS`](LINGUAS) once every entry + is translated and none is fuzzy, since a release refuses anything less + ([Keeping a catalogue complete](#keeping-a-catalogue-complete)). From + then on: - the `po` Meson suite runs `msgfmt --check` on it. With gettext 0.26 or newer, as in CI's Translations job, that fails an entry whose @@ -78,6 +80,30 @@ Start from the template CI builds, so you do not need gettext 1.0: glibc loads no catalogue at all under `C` or `C.UTF-8`, so testing a translation needs a real locale generated on the machine. +## Keeping a catalogue complete + +A language in `LINGUAS` ships complete: every entry of the current +template translated, none fuzzy. `po/check-complete.sh` checks it. From +`linux/`, after generating the template: + +```sh +meson compile -C _build tablepro-pot +po/check-complete.sh +``` + +It merges each catalogue with the template as `msgmerge` would, without +fuzzy matching, prints a table of translated, fuzzy and untranslated +entries per language, and lists the missing ones on stderr. It exits 1 +when a catalogue is incomplete. + +- **Pull requests and `main`**: the Translations job of Build Linux + reports the table in its summary and warns, and never fails. As in other + GNOME apps, strings change freely until the string freeze before a + release, and the translation run after the freeze fills the catalogues. +- **A `linux-v*` tag**: the release's Translation template job fails on an + incomplete catalogue, so nothing is published. Translate the gap, or take + the language out of `LINGUAS` for that release. + ## POTFILES.in `POTFILES.in` lists every file xgettext reads. Regenerate it with: diff --git a/linux/po/check-complete.sh b/linux/po/check-complete.sh new file mode 100755 index 0000000000..fe8097cb4c --- /dev/null +++ b/linux/po/check-complete.sh @@ -0,0 +1,54 @@ +#!/bin/sh +# Compares each catalogue in LINGUAS with the template, as msgmerge would +# bring it up to date, and prints a Markdown table of what each leaves +# untranslated or fuzzy. The entries themselves go to stderr. Exits 1 when +# a catalogue is incomplete, 2 when the check cannot run. +# +# From linux/, after `meson compile -C _build tablepro-pot`: +# po/check-complete.sh [po-directory] +set -eu + +po_dir=${1:-po} +template=$po_dir/tablepro.pot +if [ ! -f "$template" ]; then + echo "$template is missing: run meson compile -C _build tablepro-pot first" >&2 + exit 2 +fi + +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT + +# One count from msgfmt --statistics, which leaves out a part that is 0. +count() { + printf '%s\n' "$1" | tr ',' '\n' | sed -n "s/^ *\([0-9][0-9]*\) $2.*/\1/p" | grep . || echo 0 +} + +# The entries msgattrib keeps, without the header entry. +entries() { + msgattrib "$@" --no-obsolete --no-location --no-wrap "$merged" \ + | awk 'BEGIN { RS = ""; ORS = "\n\n" } !/(^|\n)msgid ""\nmsgstr/' +} + +langs=$(sed -e 's/#.*//' "$po_dir/LINGUAS") +incomplete=0 +printf '| Language | Translated | Fuzzy | Untranslated |\n|---|---|---|---|\n' +for lang in $langs; do + merged=$work/$lang.po + # Without fuzzy matching a new string counts as untranslated, not as a + # guess from a similar one that a translator still has to check. + msgmerge --quiet --no-fuzzy-matching --output-file="$merged" "$po_dir/$lang.po" "$template" || exit 2 + stats=$(LC_ALL=C msgfmt --statistics --output-file=/dev/null "$merged" 2>&1) || exit 2 + translated=$(count "$stats" 'translated message') + fuzzy=$(count "$stats" 'fuzzy translation') + untranslated=$(count "$stats" 'untranslated message') + printf '| `%s` | %s | %s | %s |\n' "$lang" "$translated" "$fuzzy" "$untranslated" + if [ "$fuzzy" -ne 0 ] || [ "$untranslated" -ne 0 ]; then + incomplete=1 + { + echo "== $lang: $untranslated untranslated, $fuzzy fuzzy" + entries --untranslated + entries --only-fuzzy + } >&2 + fi +done +exit "$incomplete" From d6dd81411dbb08679a0c7848c4dc8168a3a71241 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 13:21:07 +0700 Subject: [PATCH 02/24] docs(linux): changelog fragments for #4049 --- linux/changelog.d/4049.added.1.md | 1 + linux/changelog.d/4049.added.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 linux/changelog.d/4049.added.1.md create mode 100644 linux/changelog.d/4049.added.md diff --git a/linux/changelog.d/4049.added.1.md b/linux/changelog.d/4049.added.1.md new file mode 100644 index 0000000000..7c96460fd4 --- /dev/null +++ b/linux/changelog.d/4049.added.1.md @@ -0,0 +1 @@ +A .sql file opened from Files or the command line loads into an editor tab without running. diff --git a/linux/changelog.d/4049.added.md b/linux/changelog.d/4049.added.md new file mode 100644 index 0000000000..1c53749573 --- /dev/null +++ b/linux/changelog.d/4049.added.md @@ -0,0 +1 @@ +Before an update changes the format of saved connections, query history or tabs, the old file is kept beside it. From f4cdf9e58ad98e1182d60e58b1ae35a5fd2788cc Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 13:47:00 +0700 Subject: [PATCH 03/24] fix(linux): drop unfinished history copies and hand scripts to the chosen session --- core/crates/storage/src/query_history/mod.rs | 1 + core/crates/storage/src/upgrade_copies.rs | 63 +++++++++++++++++-- linux/crates/app/src/ui/session_window/imp.rs | 2 + linux/crates/app/src/ui/session_window/mod.rs | 55 +++++++++++++--- .../ui/session_window/session_window_input.rs | 5 ++ .../app/src/ui/workspace/workspace_tabs.rs | 9 ++- 6 files changed, 122 insertions(+), 13 deletions(-) diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index f450ab171b..11ce5dc349 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -401,6 +401,7 @@ impl QueryHistory { // What the user asked to be gone goes from the copies too: each full-text index is optimized, so // no deleted word stays in its blocks, and VACUUM leaves none in free pages. async fn delete_from_upgrade_copies(&self, connection: Option) -> Result<(), StorageError> { + crate::upgrade_copies::remove_partials_blocking(&self.path)?; for copy in crate::upgrade_copies::copies_blocking(&self.path)? { let mut database = SqliteConnectOptions::new() .filename(©.path) diff --git a/core/crates/storage/src/upgrade_copies.rs b/core/crates/storage/src/upgrade_copies.rs index 8906d2c1ae..cf18c2a6c9 100644 --- a/core/crates/storage/src/upgrade_copies.rs +++ b/core/crates/storage/src/upgrade_copies.rs @@ -87,9 +87,10 @@ pub(crate) async fn pending_upgrade(pool: &SqlitePool, migrator: &Migrator) -> R // Through the connection, since a copy of the file alone leaves out what the WAL still holds. pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) -> Result { + // One a crash left behind holds the history too, and nothing else would ever delete it. + remove_partials_blocking(path)?; let copy = vacant_name(path, version, SystemTime::now())?; let partial = partial_name(©); - remove_if_present(&partial)?; // VACUUM INTO refuses a file that is not empty, and creates a missing // one from the umask: made here, it is 0600 like the database. crate::fs::create_private_file_if_missing(&partial)?; @@ -111,12 +112,15 @@ pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) source, }; // Windows flushes only a handle opened for writing. - std::fs::OpenOptions::new() + let landed = std::fs::OpenOptions::new() .write(true) .open(&partial) .and_then(|file| file.sync_all()) - .map_err(write_error)?; - std::fs::rename(&partial, ©).map_err(write_error)?; + .and_then(|()| std::fs::rename(&partial, ©)); + if let Err(error) = landed { + let _ = std::fs::remove_file(&partial); + return Err(write_error(error)); + } if let Some(directory) = copy.parent() { tablepro_core::sync_directory(directory).map_err(write_error)?; } @@ -155,6 +159,7 @@ pub fn copies_blocking(path: &Path) -> Result, StorageError> { } pub fn remove_all_blocking(path: &Path) -> Result { + remove_partials_blocking(path)?; let copies = copies_blocking(path)?; for copy in &copies { remove_if_present(©.path)?; @@ -163,6 +168,36 @@ pub fn remove_all_blocking(path: &Path) -> Result { } // The copy just made stays even when a clock set back dates it before the others. +// Copies of a database that never finished, as `..v-.partial`. +pub fn remove_partials_blocking(path: &Path) -> Result<(), StorageError> { + let Some(name) = path.file_name().map(|name| name.to_string_lossy().into_owned()) else { + return Ok(()); + }; + let parent = parent_of(path); + let entries = match std::fs::read_dir(parent) { + Ok(entries) => entries, + Err(error) if error.kind() == ErrorKind::NotFound => return Ok(()), + Err(source) => { + return Err(StorageError::Io { + path: parent.to_owned(), + source, + }); + } + }; + let prefix = format!(".{name}.v"); + for entry in entries { + let entry = entry.map_err(|source| StorageError::Io { + path: parent.to_owned(), + source, + })?; + let found = entry.file_name().to_string_lossy().into_owned(); + if found.starts_with(&prefix) && found.ends_with(".partial") { + remove_if_present(&entry.path())?; + } + } + Ok(()) +} + fn prune_blocking(path: &Path, made: &Path) -> Result<(), StorageError> { let older: Vec = copies_blocking(path)? .into_iter() @@ -415,6 +450,26 @@ mod tests { ); } + #[tokio::test] + async fn a_copy_a_crash_left_unfinished_is_removed_before_the_next() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("items.db"); + let abandoned = root.path().join(".items.db.v1-100.partial"); + std::fs::write(&abandoned, b"half a copy").expect("seed"); + let unrelated = root.path().join(".other.db.v1-100.partial"); + std::fs::write(&unrelated, b"another file's").expect("seed"); + let pool = database(&path).await; + base().run(&pool).await.expect("the older schema"); + + keep_database(&pool, &path, 1).await.expect("a copy"); + + assert!(!abandoned.exists(), "the unfinished copy stayed"); + assert!(unrelated.exists(), "another file's copy went"); + std::fs::write(&abandoned, b"half a copy").expect("seed"); + remove_all_blocking(&path).expect("remove"); + assert!(!abandoned.exists(), "a purge left the unfinished copy"); + } + #[tokio::test] async fn a_new_current_or_newer_database_needs_no_copy() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/linux/crates/app/src/ui/session_window/imp.rs b/linux/crates/app/src/ui/session_window/imp.rs index ad1835aba0..badb741aed 100644 --- a/linux/crates/app/src/ui/session_window/imp.rs +++ b/linux/crates/app/src/ui/session_window/imp.rs @@ -105,6 +105,8 @@ pub struct SessionWindow { // Read while no relational session was in front; the next one the // window adopts opens them. pub(super) waiting_scripts: RefCell>, + // Handed over by a picker whose user chose a session this window opens; they wait for that session. + pub(super) scripts_for: RefCell>, /// Reads which filesystem a database file is on. `None` reads the /// real one; a test stands in for a network share. pub(super) classify_location: Cell>, diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index f4b2aadba1..98d227b00b 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -443,6 +443,7 @@ impl SessionWindow { SessionWindowInput::FileResolved(resolved) => self.on_file_resolved(*resolved), SessionWindowInput::OpenScript(file) => self.on_open_script(file), SessionWindowInput::ScriptLoaded { file, text } => self.on_script_loaded(file, text), + SessionWindowInput::ScriptsFor { key, scripts } => self.on_scripts_for(key, scripts), SessionWindowInput::DropWaitingScripts => { self.imp().waiting_scripts.borrow_mut().clear(); self.show_waiting_scripts(); @@ -833,6 +834,15 @@ impl SessionWindow { self.render(); shell.add_session(&workspace); workspace.start(); + let handed: Vec<(gio::File, String)> = { + let mut scripts_for = self.imp().scripts_for.borrow_mut(); + let (handed, others) = std::mem::take(&mut *scripts_for) + .into_iter() + .partition(|(owner, ..)| *owner == key); + *scripts_for = others; + handed.into_iter().map(|(_, file, text)| (file, text)).collect() + }; + self.open_scripts_in(&workspace, handed); if workspace.data_model() == Some(DataModel::Relational) { self.open_waiting_scripts(&workspace); } @@ -890,7 +900,7 @@ impl SessionWindow { let opened_for_file = self.imp().opened_for_file.replace(false); let presented = self.present_owner(window_id, &key); if presented { - self.hand_waiting_scripts_to(window_id); + self.hand_waiting_scripts_to(window_id, &key); } if presented && opened_for_file { self.close(); @@ -1187,8 +1197,8 @@ impl SessionWindow { } } - // The connection the user picked for them is in front there now. - fn hand_waiting_scripts_to(&self, window_id: u32) { + // The session the user chose for them is open, or still connecting, in another window. + fn hand_waiting_scripts_to(&self, window_id: u32, key: &SessionKey) { let Some(owner) = self .application() .and_then(|app| app.window_by_id(window_id)) @@ -1196,16 +1206,45 @@ impl SessionWindow { else { return; }; - let waiting = self.imp().waiting_scripts.take(); - if waiting.is_empty() { + let scripts = self.imp().waiting_scripts.take(); + if scripts.is_empty() { return; } - for (file, text) in waiting { - owner.send(SessionWindowInput::ScriptLoaded { file, text: Ok(text) }); - } + owner.send(SessionWindowInput::ScriptsFor { + key: key.clone(), + scripts, + }); self.show_waiting_scripts(); } + fn on_scripts_for(&self, key: SessionKey, scripts: Vec<(gio::File, String)>) { + match self + .workspaces() + .into_iter() + .find(|workspace| workspace.key().as_ref() == Some(&key)) + { + Some(workspace) => self.open_scripts_in(&workspace, scripts), + None => self + .imp() + .scripts_for + .borrow_mut() + .extend(scripts.into_iter().map(|(file, text)| (key.clone(), file, text))), + } + } + + // A session that runs no SQL leaves them waiting for one that does. + fn open_scripts_in(&self, workspace: &Workspace, scripts: Vec<(gio::File, String)>) { + let input = workspace + .sender() + .filter(|_| workspace.data_model() == Some(DataModel::Relational)); + for (file, text) in scripts { + match &input { + Some(input) => input.deliver(WorkspaceInput::OpenScript { file, text }), + None => self.wait_for_session(file, text), + } + } + } + fn open_waiting_scripts(&self, workspace: &Workspace) { let Some(input) = workspace.sender() else { return; diff --git a/linux/crates/app/src/ui/session_window/session_window_input.rs b/linux/crates/app/src/ui/session_window/session_window_input.rs index a102577c6a..61a786fc67 100644 --- a/linux/crates/app/src/ui/session_window/session_window_input.rs +++ b/linux/crates/app/src/ui/session_window/session_window_input.rs @@ -34,6 +34,11 @@ pub(crate) enum SessionWindowInput { }, /// The waiting scripts banner's Cancel. DropWaitingScripts, + /// Scripts another window's user chose this window's session for. + ScriptsFor { + key: SessionKey, + scripts: Vec<(gio::File, String)>, + }, /// Open one of the sessions the window was opened to restore. Restore(SessionKey), /// The saved connections could not be read for this restored key, diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index 86fa361741..a058ec58e9 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -232,8 +232,10 @@ impl WorkspaceModel { } pub(super) fn editor_tab_for(&self, file: &gio::File) -> Option { + let wanted = canonical(file); + let same = |bound: &gio::File| bound.equal(file) || wanted.is_some() && canonical(bound) == wanted; self.workspace_tabs.borrow().iter().find_map(|(id, tab)| match tab { - WorkspaceTab::Editor(slot) if slot.editor.file().is_some_and(|bound| bound.equal(file)) => Some(*id), + WorkspaceTab::Editor(slot) if slot.editor.file().is_some_and(|bound| same(&bound)) => Some(*id), _ => None, }) } @@ -848,6 +850,11 @@ fn saved_drafts(record: &WorkspaceRecord) -> impl Iterator + '_ }) } +// A link and its target are one file, so a tab bound to either is the one for both. +fn canonical(file: &gio::File) -> Option { + file.path().and_then(|path| std::fs::canonicalize(path).ok()) +} + #[cfg(test)] mod tests { use gtk4::subclass::prelude::*; From 16affd9db6b4deaa4084a825e6d1c4b3fded0d73 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 13:52:13 +0700 Subject: [PATCH 04/24] fix(linux): pass the header to content_type_guess as an option --- linux/crates/app/src/services/file_resolution.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/crates/app/src/services/file_resolution.rs b/linux/crates/app/src/services/file_resolution.rs index 08fa408a46..19dbce93ff 100644 --- a/linux/crates/app/src/services/file_resolution.rs +++ b/linux/crates/app/src/services/file_resolution.rs @@ -106,7 +106,7 @@ fn is_script_blocking(path: &Path, drivers: &DriverRegistry) -> bool { Err(DriverError::Busy(_)) => return false, Err(_) => Vec::new(), }; - let (content_type, _) = gio::content_type_guess(Some(path), &header); + let (content_type, _) = gio::content_type_guess(Some(path), Some(header.as_slice())); // The suffix covers a system without shared-mime-info. let named_sql = gio::content_type_is_a(&content_type, SQL_CONTENT_TYPE) || path From 16bdbdd0c970fbebda66ff6b2a74bde0c3f4f854 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 14:22:33 +0700 Subject: [PATCH 05/24] fix(linux): remote .sql files, plural forms in the catalogue check, no file reads on the GTK thread --- linux/ARCHITECTURE.md | 2 +- linux/README.md | 3 +- .../app/src/services/file_resolution.rs | 42 +++++++++++++++---- .../app/src/ui/workspace/workspace_tabs.rs | 10 +---- linux/po/README.md | 2 +- linux/po/check-complete.sh | 10 +++++ 6 files changed, 51 insertions(+), 18 deletions(-) diff --git a/linux/ARCHITECTURE.md b/linux/ARCHITECTURE.md index 9fb1c83644..2a35166954 100644 --- a/linux/ARCHITECTURE.md +++ b/linux/ARCHITECTURE.md @@ -75,7 +75,7 @@ One second runtime remains. Relm4 runs a component's commands (`sender.command`, A quit freezes the record before it asks the first window, so each window it closes stays on it, and writes it before the process may end. An approved quit then empties the list in memory; a cancelled one keeps only the windows still showing or connecting their session. Shutdown freezes it too: windows that go with the process are not the user closing them. An approved quit can leave the process running while a session closes, and an activate that reaches it then restores the record as a new launch would. -Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`). It goes to the most recently focused window, or to a new picker, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no session in front, the script waits behind a banner on the picker until the first relational session joins the window; with a key-value or document session in front, it waits in a new picker window. A tab already bound to the file comes forward instead. +Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`). It goes to the most recently focused window, or to a new picker, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no session in front, the script waits behind a banner on the picker until the first relational session joins the window; with a key-value or document session in front, it waits in a new picker window. A tab already bound to the same location comes forward instead; the lookup reads no file system on the GTK thread, so a tab bound through another path, or one still being restored, is not found. A file GIO reaches without a local path, such as over SFTP, is a script by its `.sql` name. ## Windows and sessions diff --git a/linux/README.md b/linux/README.md index 560f2a7628..d0ba269d4c 100644 --- a/linux/README.md +++ b/linux/README.md @@ -257,7 +257,8 @@ neither engine's locking can be trusted there. A SQL file opens in a new editor tab of the connection in front of the last window you used, and Save writes back to it. It never runs by itself. With no SQL connection in front, the window asks you to choose -one first. A file already open in a tab brings that tab forward. +one first. A file already open in a tab under the same path brings that +tab forward. ## Vim keybindings diff --git a/linux/crates/app/src/services/file_resolution.rs b/linux/crates/app/src/services/file_resolution.rs index 19dbce93ff..ec3390d7a2 100644 --- a/linux/crates/app/src/services/file_resolution.rs +++ b/linux/crates/app/src/services/file_resolution.rs @@ -62,6 +62,13 @@ pub fn resolve_file_blocking( connections: &ConnectionStore, drivers: &DriverRegistry, ) -> ResolvedFile { + // A file GIO reaches without a local path, such as one over SFTP, is read as text by its name. + if path.is_none() && named_sql(Path::new(&uri)) { + return ResolvedFile { + uri, + target: Ok(FileTarget::Script), + }; + } if let Some(script) = path.as_deref().filter(|path| is_script_blocking(path, drivers)) { // One file reached through a link and through its target opens one tab, as a database opens one session. let uri = std::fs::canonicalize(script) @@ -107,19 +114,20 @@ fn is_script_blocking(path: &Path, drivers: &DriverRegistry) -> bool { Err(_) => Vec::new(), }; let (content_type, _) = gio::content_type_guess(Some(path), Some(header.as_slice())); - // The suffix covers a system without shared-mime-info. - let named_sql = gio::content_type_is_a(&content_type, SQL_CONTENT_TYPE) - || path - .extension() - .and_then(|extension| extension.to_str()) - .is_some_and(|extension| extension.eq_ignore_ascii_case(SQL_SUFFIX)); - named_sql + (gio::content_type_is_a(&content_type, SQL_CONTENT_TYPE) || named_sql(path)) && !drivers .iter() .filter_map(|driver| driver.capabilities().file_format()) .any(|format| format.signature.matches(&header)) } +// The suffix covers a system without shared-mime-info. +fn named_sql(path: &Path) -> bool { + path.extension() + .and_then(|extension| extension.to_str()) + .is_some_and(|extension| extension.eq_ignore_ascii_case(SQL_SUFFIX)) +} + const SQL_CONTENT_TYPE: &str = "application/sql"; const SQL_SUFFIX: &str = "sql"; @@ -451,6 +459,26 @@ mod tests { assert_eq!(gio::File::for_uri(&resolved.uri).path(), Some(canonical)); } + #[test] + fn a_remote_sql_file_with_no_local_path_is_a_script() { + let root = tempfile::tempdir().expect("tempdir"); + let store = ConnectionStore::new(&tablepro_storage::StoragePaths::under( + root.path(), + "tablepro-test", + "app.tablepro.Test", + )); + let uri = "sftp://db.example/home/ada/report.SQL".to_owned(); + + let resolved = resolve_file_blocking(None, uri.clone(), &store, &drivers(&FakeDriver::FILE_CAPABILITIES)); + + assert!( + matches!(resolved.target, Ok(FileTarget::Script)), + "{:?}", + resolved.target + ); + assert_eq!(resolved.uri, uri); + } + // The editor's read then says why it cannot be opened. #[test] fn a_missing_sql_file_is_still_a_script() { diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index a058ec58e9..64b33133f4 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -231,11 +231,10 @@ impl WorkspaceModel { }); } + // By location only: the GTK thread reads no file system, so a tab bound through another path is not found. pub(super) fn editor_tab_for(&self, file: &gio::File) -> Option { - let wanted = canonical(file); - let same = |bound: &gio::File| bound.equal(file) || wanted.is_some() && canonical(bound) == wanted; self.workspace_tabs.borrow().iter().find_map(|(id, tab)| match tab { - WorkspaceTab::Editor(slot) if slot.editor.file().is_some_and(|bound| same(&bound)) => Some(*id), + WorkspaceTab::Editor(slot) if slot.editor.file().is_some_and(|bound| bound.equal(file)) => Some(*id), _ => None, }) } @@ -850,11 +849,6 @@ fn saved_drafts(record: &WorkspaceRecord) -> impl Iterator + '_ }) } -// A link and its target are one file, so a tab bound to either is the one for both. -fn canonical(file: &gio::File) -> Option { - file.path().and_then(|path| std::fs::canonicalize(path).ok()) -} - #[cfg(test)] mod tests { use gtk4::subclass::prelude::*; diff --git a/linux/po/README.md b/linux/po/README.md index 07600387be..c88cec1e7c 100644 --- a/linux/po/README.md +++ b/linux/po/README.md @@ -83,7 +83,7 @@ Start from the template CI builds, so you do not need gettext 1.0: ## Keeping a catalogue complete A language in `LINGUAS` ships complete: every entry of the current -template translated, none fuzzy. `po/check-complete.sh` checks it. From +template translated, each plural form included, none fuzzy. `po/check-complete.sh` checks it. From `linux/`, after generating the template: ```sh diff --git a/linux/po/check-complete.sh b/linux/po/check-complete.sh index fe8097cb4c..d5872bdb51 100755 --- a/linux/po/check-complete.sh +++ b/linux/po/check-complete.sh @@ -41,6 +41,14 @@ for lang in $langs; do translated=$(count "$stats" 'translated message') fuzzy=$(count "$stats" 'fuzzy translation') untranslated=$(count "$stats" 'untranslated message') + # msgfmt counts a plural entry whose first form is filled as translated, so an empty later form counts here. + partial=$(msgattrib --translated --no-fuzzy --no-obsolete --no-wrap "$merged" | awk ' + /^msgstr\[[0-9]+\] ""$/ { pending = 1; next } + pending { if ($0 !~ /^"/) partial = 1; pending = 0 } + /^$/ { if (partial) n++; partial = 0 } + END { if (pending || partial) n++; print n + 0 }') + translated=$((translated - partial)) + untranslated=$((untranslated + partial)) printf '| `%s` | %s | %s | %s |\n' "$lang" "$translated" "$fuzzy" "$untranslated" if [ "$fuzzy" -ne 0 ] || [ "$untranslated" -ne 0 ]; then incomplete=1 @@ -48,6 +56,8 @@ for lang in $langs; do echo "== $lang: $untranslated untranslated, $fuzzy fuzzy" entries --untranslated entries --only-fuzzy + msgattrib --translated --no-fuzzy --no-obsolete --no-wrap "$merged" \ + | awk 'BEGIN { RS = ""; ORS = "\n\n" } /msgstr\[[0-9]+\] ""(\n[^"]|$)/' } >&2 fi done From 4b3bfb9b9314f2ce3c5cc216b53df0d8f914145f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 14:27:19 +0700 Subject: [PATCH 06/24] fix(linux): gate the PPA on complete catalogues and re-ask for scripts a non-SQL session cannot take --- .github/workflows/release-linux.yml | 6 ++- linux/crates/app/src/ui/session_window/mod.rs | 47 +++++++++---------- 2 files changed, 25 insertions(+), 28 deletions(-) diff --git a/.github/workflows/release-linux.yml b/.github/workflows/release-linux.yml index 3a10dd149a..d011ae7180 100644 --- a/.github/workflows/release-linux.yml +++ b/.github/workflows/release-linux.yml @@ -224,10 +224,12 @@ jobs: ppa: name: Upload to the Launchpad PPA - needs: [verify, packages, revision-packages] + needs: [verify, packages, template, revision-packages] + # A tag publishes only once every shipped catalogue is complete (template). if: >- !cancelled() && ( - (needs.packages.result == 'success' && needs.verify.outputs.prerelease == 'false') || + (needs.packages.result == 'success' && needs.template.result == 'success' && + needs.verify.outputs.prerelease == 'false') || needs.revision-packages.result == 'success') runs-on: ubuntu-24.04 environment: linux-release diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index 98d227b00b..94108043ca 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -843,9 +843,10 @@ impl SessionWindow { handed.into_iter().map(|(_, file, text)| (file, text)).collect() }; self.open_scripts_in(&workspace, handed); - if workspace.data_model() == Some(DataModel::Relational) { - self.open_waiting_scripts(&workspace); - } + // The picker that asked is gone from view, so a session that runs no SQL hands the question on. + let waiting = self.imp().waiting_scripts.take(); + self.show_waiting_scripts(); + self.open_scripts_in(&workspace, waiting); self.sync_actions(); for warning in warnings { self.show_toast(&warning); @@ -1170,7 +1171,7 @@ impl SessionWindow { match front { Some(input) => input.deliver(WorkspaceInput::OpenScript { file, text }), None if self.page() != SessionPage::Shell => self.wait_for_session(file, text), - None => self.open_script_elsewhere(file, text), + None => self.ask_elsewhere(vec![(file, text)]), } } @@ -1185,15 +1186,21 @@ impl SessionWindow { // A session that holds keys or documents runs no SQL, so the script // asks for a connection in a window of its own. - fn open_script_elsewhere(&self, file: gio::File, text: String) { + fn ask_elsewhere(&self, scripts: Vec<(gio::File, String)>) { + if scripts.is_empty() { + return; + } let window = self .application() .and_downcast::() .and_then(|app| app.open_window(WindowLaunch::Picker)) .and_downcast::(); - match window { - Some(window) => window.send(SessionWindowInput::ScriptLoaded { file, text: Ok(text) }), - None => tracing::warn!("no window to open a SQL file in"), + let Some(window) = window else { + tracing::warn!("no window to open a SQL file in"); + return; + }; + for (file, text) in scripts { + window.send(SessionWindowInput::ScriptLoaded { file, text: Ok(text) }); } } @@ -1237,28 +1244,16 @@ impl SessionWindow { let input = workspace .sender() .filter(|_| workspace.data_model() == Some(DataModel::Relational)); - for (file, text) in scripts { - match &input { - Some(input) => input.deliver(WorkspaceInput::OpenScript { file, text }), - None => self.wait_for_session(file, text), + match input { + Some(input) => { + for (file, text) in scripts { + input.deliver(WorkspaceInput::OpenScript { file, text }); + } } + None => self.ask_elsewhere(scripts), } } - fn open_waiting_scripts(&self, workspace: &Workspace) { - let Some(input) = workspace.sender() else { - return; - }; - let waiting = self.imp().waiting_scripts.take(); - if waiting.is_empty() { - return; - } - for (file, text) in waiting { - input.deliver(WorkspaceInput::OpenScript { file, text }); - } - self.show_waiting_scripts(); - } - fn show_waiting_scripts(&self) { let Some(picker) = self.imp().picker.get() else { return; From a3852c0238fed9e977010ec9896b41a6a3847db5 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 14:33:14 +0700 Subject: [PATCH 07/24] fix(linux): scripts handed to a window still connecting wait where its picker shows them --- linux/crates/app/src/ui/session_window/imp.rs | 2 -- linux/crates/app/src/ui/session_window/mod.rs | 21 +++++++------------ 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/linux/crates/app/src/ui/session_window/imp.rs b/linux/crates/app/src/ui/session_window/imp.rs index badb741aed..ad1835aba0 100644 --- a/linux/crates/app/src/ui/session_window/imp.rs +++ b/linux/crates/app/src/ui/session_window/imp.rs @@ -105,8 +105,6 @@ pub struct SessionWindow { // Read while no relational session was in front; the next one the // window adopts opens them. pub(super) waiting_scripts: RefCell>, - // Handed over by a picker whose user chose a session this window opens; they wait for that session. - pub(super) scripts_for: RefCell>, /// Reads which filesystem a database file is on. `None` reads the /// real one; a test stands in for a network share. pub(super) classify_location: Cell>, diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index 94108043ca..6a7a161d6c 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -834,15 +834,6 @@ impl SessionWindow { self.render(); shell.add_session(&workspace); workspace.start(); - let handed: Vec<(gio::File, String)> = { - let mut scripts_for = self.imp().scripts_for.borrow_mut(); - let (handed, others) = std::mem::take(&mut *scripts_for) - .into_iter() - .partition(|(owner, ..)| *owner == key); - *scripts_for = others; - handed.into_iter().map(|(_, file, text)| (file, text)).collect() - }; - self.open_scripts_in(&workspace, handed); // The picker that asked is gone from view, so a session that runs no SQL hands the question on. let waiting = self.imp().waiting_scripts.take(); self.show_waiting_scripts(); @@ -1224,6 +1215,8 @@ impl SessionWindow { self.show_waiting_scripts(); } + // Still connecting, they wait as on this window's own picker: for the next SQL session to join, or + // behind its banner if the connect is cancelled or fails. fn on_scripts_for(&self, key: SessionKey, scripts: Vec<(gio::File, String)>) { match self .workspaces() @@ -1231,11 +1224,11 @@ impl SessionWindow { .find(|workspace| workspace.key().as_ref() == Some(&key)) { Some(workspace) => self.open_scripts_in(&workspace, scripts), - None => self - .imp() - .scripts_for - .borrow_mut() - .extend(scripts.into_iter().map(|(file, text)| (key.clone(), file, text))), + None => { + for (file, text) in scripts { + self.wait_for_session(file, text); + } + } } } From e7225e6220374aa8eee9fbb84dbbd367d64748e1 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 14:41:59 +0700 Subject: [PATCH 08/24] fix(linux): a script a restoring session holds in a saved tab waits for that tab --- linux/ARCHITECTURE.md | 2 +- .../crates/app/src/application/open_files.rs | 44 ++++++++++++++++++ linux/crates/app/src/application/tests.rs | 45 +++++++++++++++++++ linux/crates/app/src/ui/session_window/mod.rs | 19 ++++++++ linux/crates/app/src/ui/workspace/mod.rs | 5 +++ linux/crates/app/src/workspace/mod.rs | 1 + .../app/src/workspace/workspace_store.rs | 20 +++++++++ 7 files changed, 135 insertions(+), 1 deletion(-) diff --git a/linux/ARCHITECTURE.md b/linux/ARCHITECTURE.md index 2a35166954..7efaf00355 100644 --- a/linux/ARCHITECTURE.md +++ b/linux/ARCHITECTURE.md @@ -75,7 +75,7 @@ One second runtime remains. Relm4 runs a component's commands (`sender.command`, A quit freezes the record before it asks the first window, so each window it closes stays on it, and writes it before the process may end. An approved quit then empties the list in memory; a cancelled one keeps only the windows still showing or connecting their session. Shutdown freezes it too: windows that go with the process are not the user closing them. An approved quit can leave the process running while a session closes, and an activate that reaches it then restores the record as a new launch would. -Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`). It goes to the most recently focused window, or to a new picker, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no session in front, the script waits behind a banner on the picker until the first relational session joins the window; with a key-value or document session in front, it waits in a new picker window. A tab already bound to the same location comes forward instead; the lookup reads no file system on the GTK thread, so a tab bound through another path, or one still being restored, is not found. A file GIO reaches without a local path, such as over SFTP, is a script by its `.sql` name. +Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`). It goes to the most recently focused window, or to a new picker, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no session in front, the script waits behind a banner on the picker until the first relational session joins the window; with a key-value or document session in front, it waits in a new picker window. A tab already bound to the same location comes forward instead, and a session still coming back whose saved tabs hold the file keeps it for its own tab, read from its record off the GTK thread. The lookup goes by location only, so a tab bound through another path is not found. A file GIO reaches without a local path, such as over SFTP, is a script by its `.sql` name. ## Windows and sessions diff --git a/linux/crates/app/src/application/open_files.rs b/linux/crates/app/src/application/open_files.rs index b457dab2fc..da62a05635 100644 --- a/linux/crates/app/src/application/open_files.rs +++ b/linux/crates/app/src/application/open_files.rs @@ -5,6 +5,7 @@ use tablepro_session::SessionKey; use super::{TableProApplication, WindowLaunch}; use crate::services::file_resolution::{FileTarget, ResolvedFile, resolve_file_blocking}; use crate::ui::{SessionWindow, SessionWindowInput}; +use crate::workspace::saved_editor_files_blocking; impl TableProApplication { /// Open each file the file manager or the command line hands over. @@ -88,6 +89,49 @@ impl TableProApplication { window.present(); return; } + let restoring: Vec<(SessionWindow, std::path::PathBuf)> = match self.services() { + Some(services) => windows + .iter() + .flat_map(|window| { + window + .restoring_keys() + .into_iter() + .map(|key| (window.clone(), services.storage.workspaces().saved_tabs_path(&key))) + }) + .collect(), + None => Vec::new(), + }; + let Some(services) = self.services().filter(|_| !restoring.is_empty()) else { + self.route_script(windows, file); + return; + }; + // A session still coming back may hold the file in a saved tab, which a tab opened now would + // duplicate; its record says so before it connects. + let uri = file.uri().to_string(); + let paths: Vec = restoring.iter().map(|(_, path)| path.clone()).collect(); + let reading = services.tasks.spawn_blocking_task(move || { + paths + .iter() + .position(|path| saved_editor_files_blocking(path).contains(&uri)) + }); + let hold = self.hold(); + let app = self.downgrade(); + glib::spawn_future_local(async move { + let owner = reading.await.ok().flatten(); + if let Some(app) = app.upgrade() { + match owner.and_then(|index| restoring.get(index)) { + Some((window, _)) => { + window.present(); + window.tell_script_is_being_restored(&file); + } + None => app.route_script(windows, file), + } + } + drop(hold); + }); + } + + fn route_script(&self, windows: Vec, file: gio::File) { let window = match windows.into_iter().next() { Some(window) => Some(window), None => self.open_window(WindowLaunch::Picker).and_downcast::(), diff --git a/linux/crates/app/src/application/tests.rs b/linux/crates/app/src/application/tests.rs index 3a0e7880e9..6fc7b05789 100644 --- a/linux/crates/app/src/application/tests.rs +++ b/linux/crates/app/src/application/tests.rs @@ -418,6 +418,51 @@ fn a_sql_file_with_no_window_opens_a_picker_that_asks() { assert_eq!(picker().map(|window| window.page_name()), Some("picker")); } +// The launch reads the record before the session reconnects, so the script does not open a second tab. +#[gtk4::test] +fn a_sql_file_a_restored_session_holds_in_a_saved_tab_opens_no_second_tab() { + let test = TestApp::started("ScriptInRestoredTab"); + let first = test.database("first.db"); + test.record(std::slice::from_ref(&first)); + let SessionKey::File(script_uri) = test.file_key("report.sql") else { + unreachable!("a file in the directory has a file key"); + }; + std::fs::write(test._backing.dir.path().join("report.sql"), "SELECT 1;").expect("write the script"); + let saved = crate::workspace::WorkspaceRecord { + active_tab: Some(0), + tabs: vec![crate::workspace::TabRecord::Editor { + draft: tablepro_storage::DraftId::new(), + file: Some(crate::workspace::EditorFileRecord { + uri: script_uri.clone(), + modified: false, + }), + database: None, + }], + }; + tablepro_storage::document::write_document_blocking( + &test.services().storage.workspaces().saved_tabs_path(&first), + &saved, + ) + .expect("the saved tabs"); + let script = gio::File::for_uri(&script_uri); + + test.app.open(std::slice::from_ref(&script), ""); + test.wait_for_sessions(std::slice::from_ref(&first)); + let bound = || { + test.app + .windows() + .into_iter() + .filter_map(|window| window.downcast::().ok()) + .filter(|window| window.script_tab(&script).is_some()) + .count() + }; + wait_until(WAIT, || bound() == 1).expect("the restored tab bound to the file"); + let _ = wait_until(std::time::Duration::from_millis(300), || false); + + assert_eq!(bound(), 1, "the script opened beside its restored tab"); + assert_eq!(test.app.windows().len(), 1, "the script opened another window"); +} + #[gtk4::test] fn activate_restores_nothing_when_restore_session_false() { let test = TestApp::started("NoRestore"); diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index 6a7a161d6c..b46c8044ee 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -1121,6 +1121,25 @@ impl SessionWindow { } } + // The sessions whose saved tabs are not back yet: still to connect, or connected and reading them. + pub(crate) fn restoring_keys(&self) -> Vec { + let mut keys = self.imp().restoring.borrow().clone(); + keys.extend( + self.workspaces() + .iter() + .filter(|workspace| workspace.tabs_loading()) + .filter_map(Workspace::key), + ); + keys + } + + pub(crate) fn tell_script_is_being_restored(&self, file: &gio::File) { + self.show_toast(&crate::i18n::gettext_f( + "“{name}” is in a saved tab that is still coming back.", + &[("name", &file_display_name(&file.uri()))], + )); + } + pub(crate) fn script_tab(&self, file: &gio::File) -> Option { self.workspaces() .iter() diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index c37b76bc1d..495462f750 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -915,6 +915,11 @@ impl Workspace { /// The editor tab bound to `file`. `None` while the workspace is /// handling input. + // Until then a script waits in the workspace, and its saved tabs may hold the file. + pub(crate) fn tabs_loading(&self) -> bool { + self.read(|model| model.loading_scripts.is_some()).unwrap_or(false) + } + pub(crate) fn editor_tab_for(&self, file: &gio::File) -> Option { self.read(|model| model.editor_tab_for(file)).flatten() } diff --git a/linux/crates/app/src/workspace/mod.rs b/linux/crates/app/src/workspace/mod.rs index fed49aefd4..831e3dc9cd 100644 --- a/linux/crates/app/src/workspace/mod.rs +++ b/linux/crates/app/src/workspace/mod.rs @@ -125,3 +125,4 @@ pub use workspace_record::WorkspaceRecord; pub use workspace_record_builder::workspace_record; pub use workspace_save_error::WorkspaceSaveError; pub use workspace_store::WorkspaceStore; +pub(crate) use workspace_store::saved_editor_files_blocking; diff --git a/linux/crates/app/src/workspace/workspace_store.rs b/linux/crates/app/src/workspace/workspace_store.rs index a4f8a5a81b..da8730ce87 100644 --- a/linux/crates/app/src/workspace/workspace_store.rs +++ b/linux/crates/app/src/workspace/workspace_store.rs @@ -272,6 +272,11 @@ impl WorkspaceStore { } } + // Where the session's saved tabs are, for a read off the GTK thread. + pub(crate) fn saved_tabs_path(&self, key: &SessionKey) -> PathBuf { + self.inner.path(key) + } + pub fn is_blocked(&self, key: &SessionKey) -> bool { self.inner.blocked.borrow().contains(key) } @@ -497,6 +502,21 @@ fn read_record(path: &Path) -> WorkspaceLoad { } } +// The files the saved editor tabs at `path` are bound to, by URI; none when the record cannot be read. +pub(crate) fn saved_editor_files_blocking(path: &Path) -> Vec { + match read_record(path) { + WorkspaceLoad::Restored(record) => record + .tabs + .into_iter() + .filter_map(|tab| match tab { + super::TabRecord::Editor { file: Some(file), .. } => Some(file.uri), + _ => None, + }) + .collect(), + WorkspaceLoad::Empty | WorkspaceLoad::Blocked(_) => Vec::new(), + } +} + /// The whole file, or `None` when it is larger than `MAX_FILE_BYTES`. pub(super) fn read_capped(path: &Path) -> std::io::Result>> { let file = std::fs::File::open(path)?; From 3c99a5dac5d56ef80969dcdd00bacb0937ad2678 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 14:49:50 +0700 Subject: [PATCH 09/24] fix(linux): ask for a connection while windows settle instead of guessing a script's session --- linux/ARCHITECTURE.md | 2 +- linux/README.md | 6 +-- .../crates/app/src/application/open_files.rs | 47 +++-------------- linux/crates/app/src/application/tests.rs | 50 +++++++------------ linux/crates/app/src/ui/session_window/mod.rs | 44 ++++++++-------- .../src/ui/session_window/tests/open_file.rs | 33 ++++++++---- linux/crates/app/src/workspace/mod.rs | 1 - .../app/src/workspace/workspace_store.rs | 20 -------- 8 files changed, 76 insertions(+), 127 deletions(-) diff --git a/linux/ARCHITECTURE.md b/linux/ARCHITECTURE.md index 7efaf00355..f2801d1f59 100644 --- a/linux/ARCHITECTURE.md +++ b/linux/ARCHITECTURE.md @@ -75,7 +75,7 @@ One second runtime remains. Relm4 runs a component's commands (`sender.command`, A quit freezes the record before it asks the first window, so each window it closes stays on it, and writes it before the process may end. An approved quit then empties the list in memory; a cancelled one keeps only the windows still showing or connecting their session. Shutdown freezes it too: windows that go with the process are not the user closing them. An approved quit can leave the process running while a session closes, and an activate that reaches it then restores the record as a new launch would. -Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`). It goes to the most recently focused window, or to a new picker, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no session in front, the script waits behind a banner on the picker until the first relational session joins the window; with a key-value or document session in front, it waits in a new picker window. A tab already bound to the same location comes forward instead, and a session still coming back whose saved tabs hold the file keeps it for its own tab, read from its record off the GTK thread. The lookup goes by location only, so a tab bound through another path is not found. A file GIO reaches without a local path, such as over SFTP, is a script by its `.sql` name. +Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`); one GIO reaches without a local path, such as over SFTP, is a script by its name. A tab already bound to the same location comes forward; the lookup goes by location, so the GTK thread reads no file system. Otherwise the script goes to the most recently focused window, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no relational session in front, or while any window is still restoring, connecting or reading its saved tabs, where a tab bound to the file may yet appear, a picker asks: the script waits behind its banner until a relational session joins that window. A session that runs no SQL hands the question to a new picker, and one chosen while it is open in another window takes the scripts there. ## Windows and sessions diff --git a/linux/README.md b/linux/README.md index d0ba269d4c..ed77f7e324 100644 --- a/linux/README.md +++ b/linux/README.md @@ -256,9 +256,9 @@ neither engine's locking can be trusted there. A SQL file opens in a new editor tab of the connection in front of the last window you used, and Save writes back to it. It never runs by -itself. With no SQL connection in front, the window asks you to choose -one first. A file already open in a tab under the same path brings that -tab forward. +itself. With no SQL connection in front, or while a window is still +connecting or reopening its tabs, a window asks you to choose one first. +A file already open in a tab under the same path brings that tab forward. ## Vim keybindings diff --git a/linux/crates/app/src/application/open_files.rs b/linux/crates/app/src/application/open_files.rs index da62a05635..8a22bed9a8 100644 --- a/linux/crates/app/src/application/open_files.rs +++ b/linux/crates/app/src/application/open_files.rs @@ -89,46 +89,13 @@ impl TableProApplication { window.present(); return; } - let restoring: Vec<(SessionWindow, std::path::PathBuf)> = match self.services() { - Some(services) => windows - .iter() - .flat_map(|window| { - window - .restoring_keys() - .into_iter() - .map(|key| (window.clone(), services.storage.workspaces().saved_tabs_path(&key))) - }) - .collect(), - None => Vec::new(), - }; - let Some(services) = self.services().filter(|_| !restoring.is_empty()) else { - self.route_script(windows, file); - return; - }; - // A session still coming back may hold the file in a saved tab, which a tab opened now would - // duplicate; its record says so before it connects. - let uri = file.uri().to_string(); - let paths: Vec = restoring.iter().map(|(_, path)| path.clone()).collect(); - let reading = services.tasks.spawn_blocking_task(move || { - paths - .iter() - .position(|path| saved_editor_files_blocking(path).contains(&uri)) - }); - let hold = self.hold(); - let app = self.downgrade(); - glib::spawn_future_local(async move { - let owner = reading.await.ok().flatten(); - if let Some(app) = app.upgrade() { - match owner.and_then(|index| restoring.get(index)) { - Some((window, _)) => { - window.present(); - window.tell_script_is_being_restored(&file); - } - None => app.route_script(windows, file), - } - } - drop(hold); - }); + // While any window settles, a tab bound to the file may still appear, so a picker asks rather + // than opening a second one beside it. + if windows.iter().any(SessionWindow::settling) { + let idle = windows.into_iter().next().filter(SessionWindow::is_idle); + return self.route_script(idle.into_iter().collect(), file); + } + self.route_script(windows, file); } fn route_script(&self, windows: Vec, file: gio::File) { diff --git a/linux/crates/app/src/application/tests.rs b/linux/crates/app/src/application/tests.rs index 6fc7b05789..c88a05bdbf 100644 --- a/linux/crates/app/src/application/tests.rs +++ b/linux/crates/app/src/application/tests.rs @@ -418,49 +418,37 @@ fn a_sql_file_with_no_window_opens_a_picker_that_asks() { assert_eq!(picker().map(|window| window.page_name()), Some("picker")); } -// The launch reads the record before the session reconnects, so the script does not open a second tab. +// A restored session may hold the file in a tab that is not back yet, so a picker asks rather than +// opening a second tab beside it. #[gtk4::test] -fn a_sql_file_a_restored_session_holds_in_a_saved_tab_opens_no_second_tab() { - let test = TestApp::started("ScriptInRestoredTab"); +fn a_sql_file_opened_while_sessions_restore_asks_for_a_connection() { + let test = TestApp::started("ScriptWhileRestoring"); let first = test.database("first.db"); test.record(std::slice::from_ref(&first)); - let SessionKey::File(script_uri) = test.file_key("report.sql") else { - unreachable!("a file in the directory has a file key"); - }; - std::fs::write(test._backing.dir.path().join("report.sql"), "SELECT 1;").expect("write the script"); - let saved = crate::workspace::WorkspaceRecord { - active_tab: Some(0), - tabs: vec![crate::workspace::TabRecord::Editor { - draft: tablepro_storage::DraftId::new(), - file: Some(crate::workspace::EditorFileRecord { - uri: script_uri.clone(), - modified: false, - }), - database: None, - }], - }; - tablepro_storage::document::write_document_blocking( - &test.services().storage.workspaces().saved_tabs_path(&first), - &saved, - ) - .expect("the saved tabs"); - let script = gio::File::for_uri(&script_uri); + let path = test._backing.dir.path().join("report.sql"); + std::fs::write(&path, "SELECT 1;").expect("write the script"); + let script = gio::File::for_path(&path); test.app.open(std::slice::from_ref(&script), ""); test.wait_for_sessions(std::slice::from_ref(&first)); - let bound = || { + let windows = || { test.app .windows() .into_iter() .filter_map(|window| window.downcast::().ok()) - .filter(|window| window.script_tab(&script).is_some()) - .count() + .collect::>() }; - wait_until(WAIT, || bound() == 1).expect("the restored tab bound to the file"); - let _ = wait_until(std::time::Duration::from_millis(300), || false); + wait_until(WAIT, || { + windows() + .iter() + .any(|window| window.script_banner().is_some_and(|banner| banner.is_revealed())) + }) + .expect("a picker asking for a connection"); - assert_eq!(bound(), 1, "the script opened beside its restored tab"); - assert_eq!(test.app.windows().len(), 1, "the script opened another window"); + assert!( + windows().iter().all(|window| window.script_tab(&script).is_none()), + "the script opened in a session while it was restoring" + ); } #[gtk4::test] diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index b46c8044ee..bcb9cff7cf 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -1121,23 +1121,13 @@ impl SessionWindow { } } - // The sessions whose saved tabs are not back yet: still to connect, or connected and reading them. - pub(crate) fn restoring_keys(&self) -> Vec { - let mut keys = self.imp().restoring.borrow().clone(); - keys.extend( - self.workspaces() - .iter() - .filter(|workspace| workspace.tabs_loading()) - .filter_map(Workspace::key), - ); - keys - } - - pub(crate) fn tell_script_is_being_restored(&self, file: &gio::File) { - self.show_toast(&crate::i18n::gettext_f( - "“{name}” is in a saved tab that is still coming back.", - &[("name", &file_display_name(&file.uri()))], - )); + // Sessions still to restore, connecting, or reading their saved tabs may hold a script in a tab + // that is not there yet. + pub(crate) fn settling(&self) -> bool { + let imp = self.imp(); + !imp.restoring.try_borrow().is_ok_and(|restoring| restoring.is_empty()) + || !imp.connects.try_borrow().is_ok_and(|connects| connects.is_empty()) + || self.workspaces().iter().any(Workspace::tabs_loading) } pub(crate) fn script_tab(&self, file: &gio::File) -> Option { @@ -1214,7 +1204,8 @@ impl SessionWindow { } } - // The session the user chose for them is open, or still connecting, in another window. + // Only to a session that has joined there: one still connecting could fail or be cancelled, and + // the scripts would no longer be asked for anywhere. fn hand_waiting_scripts_to(&self, window_id: u32, key: &SessionKey) { let Some(owner) = self .application() @@ -1223,10 +1214,20 @@ impl SessionWindow { else { return; }; - let scripts = self.imp().waiting_scripts.take(); - if scripts.is_empty() { + if self.imp().waiting_scripts.borrow().is_empty() { return; } + if !owner + .workspaces() + .iter() + .any(|workspace| workspace.key().as_ref() == Some(key)) + { + self.show_toast(&crate::i18n::gettext( + "That connection is still connecting in another window. Choose it again once it is open.", + )); + return; + } + let scripts = self.imp().waiting_scripts.take(); owner.send(SessionWindowInput::ScriptsFor { key: key.clone(), scripts, @@ -1234,8 +1235,7 @@ impl SessionWindow { self.show_waiting_scripts(); } - // Still connecting, they wait as on this window's own picker: for the next SQL session to join, or - // behind its banner if the connect is cancelled or fails. + // The session left between the hand-over and now: they wait here instead, behind the picker's banner. fn on_scripts_for(&self, key: SessionKey, scripts: Vec<(gio::File, String)>) { match self .workspaces() diff --git a/linux/crates/app/src/ui/session_window/tests/open_file.rs b/linux/crates/app/src/ui/session_window/tests/open_file.rs index b58cfdefe5..6839d1c9de 100644 --- a/linux/crates/app/src/ui/session_window/tests/open_file.rs +++ b/linux/crates/app/src/ui/session_window/tests/open_file.rs @@ -124,6 +124,10 @@ fn open(test: &TestWindow, files: &[gio::File]) { .settings .set_restore_session(false) .expect("turn restoring off"); + // A window still reading its saved tabs sends the script to a picker instead. + if test.window.page_name() == "workspace" { + wait_until(WAIT, || !test.window.settling()).expect("the saved tabs read"); + } test.app.open(files, ""); } @@ -225,8 +229,9 @@ fn the_banner_cancel_drops_the_waiting_scripts() { ); } +// A session still connecting may bring the file back in a saved tab, so a picker of its own asks. #[gtk4::test] -fn a_sql_file_while_connecting_opens_once_the_session_joins() { +fn a_sql_file_while_a_window_connects_asks_in_a_picker_of_its_own() { let test = TestWindow::with_script( "ScriptConnecting", Script { @@ -243,15 +248,25 @@ fn a_sql_file_while_connecting_opens_once_the_session_joins() { let file = script(&test, "report.sql", "SELECT 1;"); open(&test, std::slice::from_ref(&file)); - wait_until(WAIT, || !test.window.imp().waiting_scripts.borrow().is_empty()) - .expect("the script to wait for the connect"); - assert_eq!(test.window.page_name(), "connecting"); - let editor = script_editor(&test.window, &file); + let asking = || { + test.app + .windows() + .into_iter() + .filter_map(|window| window.downcast::().ok()) + .find(|window| window.script_banner().is_some_and(|banner| banner.is_revealed())) + }; + wait_until(WAIT, || asking().is_some()).expect("a picker asking for a connection"); + test.wait_for_page("workspace"); + drain_main_context(); - assert_eq!(test.window.page_name(), "workspace"); - assert_eq!(text_of(&editor), "SELECT 1;"); - assert!(test.window.imp().waiting_scripts.borrow().is_empty()); - assert_eq!(test.connector.calls().len(), 1, "the script was opened as a database"); + assert!( + asking().is_some_and(|window| window != test.window), + "the connecting window took the script" + ); + assert!( + editors(&test.window).iter().all(|editor| !bound_to(editor, &file)), + "the script opened in the session that was connecting" + ); } #[gtk4::test] diff --git a/linux/crates/app/src/workspace/mod.rs b/linux/crates/app/src/workspace/mod.rs index 831e3dc9cd..fed49aefd4 100644 --- a/linux/crates/app/src/workspace/mod.rs +++ b/linux/crates/app/src/workspace/mod.rs @@ -125,4 +125,3 @@ pub use workspace_record::WorkspaceRecord; pub use workspace_record_builder::workspace_record; pub use workspace_save_error::WorkspaceSaveError; pub use workspace_store::WorkspaceStore; -pub(crate) use workspace_store::saved_editor_files_blocking; diff --git a/linux/crates/app/src/workspace/workspace_store.rs b/linux/crates/app/src/workspace/workspace_store.rs index da8730ce87..a4f8a5a81b 100644 --- a/linux/crates/app/src/workspace/workspace_store.rs +++ b/linux/crates/app/src/workspace/workspace_store.rs @@ -272,11 +272,6 @@ impl WorkspaceStore { } } - // Where the session's saved tabs are, for a read off the GTK thread. - pub(crate) fn saved_tabs_path(&self, key: &SessionKey) -> PathBuf { - self.inner.path(key) - } - pub fn is_blocked(&self, key: &SessionKey) -> bool { self.inner.blocked.borrow().contains(key) } @@ -502,21 +497,6 @@ fn read_record(path: &Path) -> WorkspaceLoad { } } -// The files the saved editor tabs at `path` are bound to, by URI; none when the record cannot be read. -pub(crate) fn saved_editor_files_blocking(path: &Path) -> Vec { - match read_record(path) { - WorkspaceLoad::Restored(record) => record - .tabs - .into_iter() - .filter_map(|tab| match tab { - super::TabRecord::Editor { file: Some(file), .. } => Some(file.uri), - _ => None, - }) - .collect(), - WorkspaceLoad::Empty | WorkspaceLoad::Blocked(_) => Vec::new(), - } -} - /// The whole file, or `None` when it is larger than `MAX_FILE_BYTES`. pub(super) fn read_capped(path: &Path) -> std::io::Result>> { let file = std::fs::File::open(path)?; From 6b10280c04c472910104b555d094493674bb9e81 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 14:50:07 +0700 Subject: [PATCH 10/24] fix(linux): drop an import the routing no longer uses --- linux/crates/app/src/application/open_files.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/linux/crates/app/src/application/open_files.rs b/linux/crates/app/src/application/open_files.rs index 8a22bed9a8..7621b2369b 100644 --- a/linux/crates/app/src/application/open_files.rs +++ b/linux/crates/app/src/application/open_files.rs @@ -5,7 +5,6 @@ use tablepro_session::SessionKey; use super::{TableProApplication, WindowLaunch}; use crate::services::file_resolution::{FileTarget, ResolvedFile, resolve_file_blocking}; use crate::ui::{SessionWindow, SessionWindowInput}; -use crate::workspace::saved_editor_files_blocking; impl TableProApplication { /// Open each file the file manager or the command line hands over. From 56f73c4a52670b8f377331dcc075912333962787 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 14:56:19 +0700 Subject: [PATCH 11/24] fix(linux): a waiting script brings forward a tab another window restored meanwhile --- linux/crates/app/src/ui/session_window/mod.rs | 23 +++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index bcb9cff7cf..b549dbbe44 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -1130,6 +1130,22 @@ impl SessionWindow { || self.workspaces().iter().any(Workspace::tabs_loading) } + // Every window: while a script waited, a tab bound to its file may have come back in another one. + fn focus_bound_tab(&self, file: &gio::File) -> bool { + let found = self.application().and_then(|app| { + app.windows() + .into_iter() + .filter_map(|window| window.downcast::().ok()) + .find_map(|window| window.script_tab(file).map(|tab| (window, tab))) + }); + let Some((window, tab)) = found else { + return false; + }; + window.send(SessionWindowInput::FocusTab(tab)); + window.present(); + true + } + pub(crate) fn script_tab(&self, file: &gio::File) -> Option { self.workspaces() .iter() @@ -1160,8 +1176,7 @@ impl SessionWindow { return; } }; - if let Some(tab) = self.script_tab(&file) { - self.on_focus_tab(tab); + if self.focus_bound_tab(&file) { return; } let front = self @@ -1253,6 +1268,10 @@ impl SessionWindow { // A session that runs no SQL leaves them waiting for one that does. fn open_scripts_in(&self, workspace: &Workspace, scripts: Vec<(gio::File, String)>) { + let scripts: Vec<(gio::File, String)> = scripts + .into_iter() + .filter(|(file, _)| !self.focus_bound_tab(file)) + .collect(); let input = workspace .sender() .filter(|_| workspace.data_model() == Some(DataModel::Relational)); From 68280c67c5cc560cf02977c81295759022e21621 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:04:25 +0700 Subject: [PATCH 12/24] fix(linux): a script released after saved tabs load looks in every window first --- linux/crates/app/src/application/open_files.rs | 7 +------ linux/crates/app/src/ui/session_window/mod.rs | 17 +++++++++++------ .../app/src/ui/workspace/workspace_tabs.rs | 9 +++++++++ 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/linux/crates/app/src/application/open_files.rs b/linux/crates/app/src/application/open_files.rs index 7621b2369b..1126780360 100644 --- a/linux/crates/app/src/application/open_files.rs +++ b/linux/crates/app/src/application/open_files.rs @@ -80,12 +80,7 @@ impl TableProApplication { .into_iter() .filter_map(|window| window.downcast::().ok()) .collect(); - if let Some((window, tab)) = windows - .iter() - .find_map(|window| Some((window, window.script_tab(&file)?))) - { - window.send(SessionWindowInput::FocusTab(tab)); - window.present(); + if SessionWindow::focus_tab_bound_to(self, &file) { return; } // While any window settles, a tab bound to the file may still appear, so a picker asks rather diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index b549dbbe44..79d92812aa 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -1132,12 +1132,17 @@ impl SessionWindow { // Every window: while a script waited, a tab bound to its file may have come back in another one. fn focus_bound_tab(&self, file: &gio::File) -> bool { - let found = self.application().and_then(|app| { - app.windows() - .into_iter() - .filter_map(|window| window.downcast::().ok()) - .find_map(|window| window.script_tab(file).map(|tab| (window, tab))) - }); + self.application() + .is_some_and(|app| Self::focus_tab_bound_to(&app, file)) + } + + // A workspace handling input answers no lookup, so the caller checks its own tabs itself. + pub(crate) fn focus_tab_bound_to(app: &impl IsA, file: &gio::File) -> bool { + let found = app + .windows() + .into_iter() + .filter_map(|window| window.downcast::().ok()) + .find_map(|window| window.script_tab(file).map(|tab| (window, tab))); let Some((window, tab)) = found else { return false; }; diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index 64b33133f4..a69e8db55d 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -220,6 +220,15 @@ impl WorkspaceModel { self.focus_tab(id); return; } + // Released after the saved tabs came back, by which time another window may hold the file. + let elsewhere = self + .window + .upgrade() + .and_then(|window| window.application()) + .is_some_and(|app| crate::ui::SessionWindow::focus_tab_bound_to(&app, file)); + if elsewhere { + return; + } self.open_editor(TabSpec::Editor { initial_query: Some(text), draft: None, From 8918f17d7cb436b70e74b192a93bc1524fe50ec8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:14:24 +0700 Subject: [PATCH 13/24] fix(core-storage): every history and saved-query deletion, and retention, reach the upgrade copies --- core/crates/storage/src/query_history/mod.rs | 177 ++++++++++++++---- core/crates/storage/src/saved_queries/mod.rs | 24 ++- .../0011-copies-before-a-format-upgrade.md | 2 +- linux/docs/storage.md | 7 +- linux/docs/troubleshooting.md | 2 +- 5 files changed, 171 insertions(+), 41 deletions(-) diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index 11ce5dc349..f013dd2faa 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -52,7 +52,7 @@ const ENTRY_COLUMNS: &str = "h.id, h.query, h.driver_id, h.connection_id, h.conn h.duration_ms, h.rows_affected, h.success, h.cancelled, h.pinned, h.error"; /// A value for a placeholder of a statement built from parts. -enum Bind { +pub(crate) enum Bind { Text(String), Integer(i64), } @@ -98,7 +98,7 @@ impl QueryHistory { /// migration chain, one open: what the user keeps and what they /// ran are the same kind of thing in the same place. pub fn saved_queries(&self) -> crate::saved_queries::SavedQueries { - crate::saved_queries::SavedQueries::new(self.pool.clone()) + crate::saved_queries::SavedQueries::new(self.pool.clone(), self.path.clone()) } pub async fn record(&self, entry: NewEntry) -> Result { @@ -264,6 +264,17 @@ impl QueryHistory { } } transaction.commit().await?; + let in_copies = ids + .chunks(IDS_PER_STATEMENT) + .map(|chunk| { + let placeholders = vec!["?"; chunk.len()].join(","); + ( + format!("DELETE FROM history WHERE id IN ({placeholders})"), + chunk.iter().map(|id| Bind::Integer(*id)).collect(), + ) + }) + .collect(); + delete_in_upgrade_copies(&self.path, "history", in_copies).await?; deleted.sort_by_key(|entry| entry.id); Ok(deleted) } @@ -311,7 +322,12 @@ impl QueryHistory { .execute(&self.pool) .await?; } - self.delete_from_upgrade_copies(None).await?; + delete_in_upgrade_copies( + &self.path, + "history", + vec![("DELETE FROM history".to_owned(), Vec::new())], + ) + .await?; Ok(affected as usize) } @@ -331,6 +347,16 @@ impl QueryHistory { .execute(&self.pool) .await? .rows_affected(); + // A copy keeps no query past the window the user chose either. + delete_in_upgrade_copies( + &self.path, + "history", + vec![( + "DELETE FROM history WHERE pinned = 0 AND executed_at < ?".to_owned(), + vec![Bind::Integer(to_unix(cutoff))], + )], + ) + .await?; } report.over_cap = sqlx::query( @@ -394,41 +420,18 @@ impl QueryHistory { .execute(&self.pool) .await?; } - self.delete_from_upgrade_copies(Some(connection)).await?; + delete_in_upgrade_copies( + &self.path, + "history", + vec![( + "DELETE FROM history WHERE connection_id = ?".to_owned(), + vec![Bind::Text(connection.to_string())], + )], + ) + .await?; Ok(removed) } - // What the user asked to be gone goes from the copies too: each full-text index is optimized, so - // no deleted word stays in its blocks, and VACUUM leaves none in free pages. - async fn delete_from_upgrade_copies(&self, connection: Option) -> Result<(), StorageError> { - crate::upgrade_copies::remove_partials_blocking(&self.path)?; - for copy in crate::upgrade_copies::copies_blocking(&self.path)? { - let mut database = SqliteConnectOptions::new() - .filename(©.path) - .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) - .disable_statement_logging() - .connect() - .await?; - let delete = match connection { - Some(connection) => { - sqlx::query("DELETE FROM history WHERE connection_id = ?").bind(connection.to_string()) - } - None => sqlx::query("DELETE FROM history"), - }; - delete.execute(&mut database).await?; - for index in full_text_indexes(&mut database).await? { - sqlx::query(AssertSqlSafe(format!( - "INSERT INTO \"{index}\"(\"{index}\") VALUES('optimize')" - ))) - .execute(&mut database) - .await?; - } - sqlx::query("VACUUM").execute(&mut database).await?; - database.close().await?; - } - Ok(()) - } - /// Every connection the history holds entries for, under the name /// of its latest run, so a renamed connection is listed once. pub async fn connections_with_history(&self) -> Result, StorageError> { @@ -480,6 +483,54 @@ impl QueryHistory { } } +// What the user deleted, or the retention window dropped, goes from the copies an upgrade kept too: +// each full-text index is optimized, so no deleted word stays in its blocks, and VACUUM leaves none in +// free pages. A copy without `table` is left alone. +pub(crate) async fn delete_in_upgrade_copies( + path: &Path, + table: &str, + deletes: Vec<(String, Vec)>, +) -> Result<(), StorageError> { + crate::upgrade_copies::remove_partials_blocking(path)?; + for copy in crate::upgrade_copies::copies_blocking(path)? { + let mut database = SqliteConnectOptions::new() + .filename(©.path) + .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .disable_statement_logging() + .connect() + .await?; + let has_table: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = ?") + .bind(table) + .fetch_one(&mut database) + .await?; + let mut removed = 0; + if has_table > 0 { + for (sql, binds) in &deletes { + let mut statement = sqlx::query(AssertSqlSafe(sql.clone())); + for bind in binds { + statement = match bind { + Bind::Text(text) => statement.bind(text.clone()), + Bind::Integer(value) => statement.bind(*value), + }; + } + removed += statement.execute(&mut database).await?.rows_affected(); + } + } + if removed > 0 { + for index in full_text_indexes(&mut database).await? { + sqlx::query(AssertSqlSafe(format!( + "INSERT INTO \"{index}\"(\"{index}\") VALUES('optimize')" + ))) + .execute(&mut database) + .await?; + } + sqlx::query("VACUUM").execute(&mut database).await?; + } + database.close().await?; + } + Ok(()) +} + // A copy holds the schema of the build that wrote it, so its indexes are found rather than named. async fn full_text_indexes(database: &mut sqlx::SqliteConnection) -> Result, StorageError> { let names: Vec = sqlx::query_scalar( @@ -1268,4 +1319,60 @@ mod tests { .expect("count"); assert_eq!(saved, 1, "a saved query went with the history"); } + + #[tokio::test] + async fn deleted_entries_expired_ones_and_deleted_saved_queries_leave_the_copies_too() { + let root = tempfile::tempdir().expect("tempdir"); + let history = QueryHistory::open(&paths(&root)).await.expect("open"); + let kept = history.record(entry("SELECT qqqq", Uuid::nil())).await.expect("record"); + let deleted = history.record(entry("SELECT zzzz", Uuid::nil())).await.expect("record"); + let mut old = entry("SELECT xxxx", Uuid::nil()); + old.executed_at = SystemTime::now() - Duration::from_secs(400 * 24 * 60 * 60); + history.record(old).await.expect("record"); + let saved = history.saved_queries(); + let named = |name: &str| crate::saved_queries::NewSavedQuery { + name: name.to_owned(), + query: "SELECT 1".to_owned(), + connection_id: Uuid::nil(), + connection_name: "test".to_owned(), + }; + saved.save(named("first")).await.expect("a saved query"); + saved.save(named("second")).await.expect("a saved query"); + let first = saved + .list(None) + .await + .expect("list") + .into_iter() + .find(|query| query.name == "first") + .expect("the first saved query") + .id; + let copy = crate::upgrade_copies::keep_database(&history.pool, &history.path, 1) + .await + .expect("a copy"); + + history.delete_returning(&[deleted]).await.expect("delete"); + history.prune(30).await.expect("prune"); + saved.delete(first).await.expect("delete a saved query"); + + assert_eq!( + copy_rows(©).await, + 1, + "a deleted or expired query stayed in the copy" + ); + assert!(!indexed(©, "zzz").await); + assert!(!indexed(©, "xxx").await); + assert!(indexed(©, "qqq").await, "a kept query went"); + let mut database = SqliteConnectOptions::new() + .filename(©) + .read_only(true) + .connect() + .await + .expect("open the copy"); + let names: Vec = sqlx::query_scalar("SELECT name FROM saved_queries") + .fetch_all(&mut database) + .await + .expect("names"); + assert_eq!(names, ["second"], "a deleted saved query stayed in the copy"); + assert!(all(&history).await.iter().any(|entry| entry.id == kept)); + } } diff --git a/core/crates/storage/src/saved_queries/mod.rs b/core/crates/storage/src/saved_queries/mod.rs index b0572de096..362b086e36 100644 --- a/core/crates/storage/src/saved_queries/mod.rs +++ b/core/crates/storage/src/saved_queries/mod.rs @@ -34,11 +34,13 @@ const MAX_NAME_CHARS: usize = 200; #[derive(Debug, Clone)] pub struct SavedQueries { pool: SqlitePool, + // The database's file, whose upgrade copies a delete reaches too. + path: std::path::PathBuf, } impl SavedQueries { - pub(crate) fn new(pool: SqlitePool) -> Self { - Self { pool } + pub(crate) fn new(pool: SqlitePool, path: std::path::PathBuf) -> Self { + Self { pool, path } } /// Keep `query` under its name, replacing whatever that name held @@ -215,6 +217,15 @@ impl SavedQueries { .execute(&self.pool) .await? .rows_affected(); + crate::query_history::delete_in_upgrade_copies( + &self.path, + "saved_queries", + vec![( + "DELETE FROM saved_queries WHERE id = ?".to_owned(), + vec![crate::query_history::Bind::Integer(id)], + )], + ) + .await?; match affected { 0 => Err(StorageError::NotFound), _ => Ok(()), @@ -230,6 +241,15 @@ impl SavedQueries { .execute(&self.pool) .await? .rows_affected(); + crate::query_history::delete_in_upgrade_copies( + &self.path, + "saved_queries", + vec![( + "DELETE FROM saved_queries WHERE connection_id = ?".to_owned(), + vec![crate::query_history::Bind::Text(connection.to_string())], + )], + ) + .await?; Ok(affected as usize) } } diff --git a/core/docs/decisions/0011-copies-before-a-format-upgrade.md b/core/docs/decisions/0011-copies-before-a-format-upgrade.md index 9afa18f585..3246f72544 100644 --- a/core/docs/decisions/0011-copies-before-a-format-upgrade.md +++ b/core/docs/decisions/0011-copies-before-a-format-upgrade.md @@ -18,7 +18,7 @@ Before the first write that replaces a file holding an older format, the storage - **Beside the file, like the copies a Reset moves aside.** `connections.json.v1-…` sits next to `connections.json.corrupt-…`, a restore is a rename in one folder, and a session's purge removes its copies with its file. - **Three per file, as Chromium keeps three user-data snapshots** taken at each major update for a rollback ([Keep data during version rollback](https://support.google.com/chrome/a/answer/9917429)). A copy whose bytes match the newest one is not made again. - **No copy, no write.** A failed copy fails the write, the file stays as it was, and the app words the error by the copy's path like any other failed write. Upgrading without the way back is what this decision removes. -- **What the user deletes is deleted from the copies.** Clear History and deleting a connection's history also delete those rows from each history copy, then `VACUUM` it; saved queries stay. +- **What the user deletes is deleted from the copies.** Every deletion from the history or the saved queries, and the history's retention window, applies to each history copy as well, which then optimizes its full-text indexes and runs `VACUUM`, so a copy never outlives the user's choice about their SQL. ## Consequences diff --git a/linux/docs/storage.md b/linux/docs/storage.md index 6793ad82c2..e12171bb45 100644 --- a/linux/docs/storage.md +++ b/linux/docs/storage.md @@ -251,8 +251,11 @@ can tell whether a re-pinned entry falls in the pages already read. - `delete_for_connection` removes one connection's entries, pinned ones included, for a connection deleted along with its history, then runs `optimize` as a prune does. -- Both delete the same rows from every copy an upgrade kept, then `VACUUM` - it, so nothing of them stays on disk. A copy keeps its saved queries. +- Every deletion, `delete_returning`, `clear_all`, `delete_for_connection`, + the retention part of `prune` and the saved queries' `delete` and + `delete_for_connection`, deletes the same rows from each copy an upgrade + kept, optimizes its full-text indexes and runs `VACUUM`, so nothing of them + stays on disk (`delete_in_upgrade_copies`). - `connections_with_history` lists each connection the history holds under the name of its latest run, so a renamed connection shows once. diff --git a/linux/docs/troubleshooting.md b/linux/docs/troubleshooting.md index 4361637c31..30b861f75b 100644 --- a/linux/docs/troubleshooting.md +++ b/linux/docs/troubleshooting.md @@ -94,7 +94,7 @@ Installing the newer release again brings all of it back, unless it was reset. ### Going back to what the older release saved -Before a release writes a file in a newer format for the first time, it keeps a copy of the file as it was, in the same folder, named after the file, the format version it holds and the time in seconds, such as `connections.json.v1-1762300000`. It keeps the newest three copies of each file. Clear History, and deleting a connection with its history, delete that history from the copies too. +Before a release writes a file in a newer format for the first time, it keeps a copy of the file as it was, in the same folder, named after the file, the format version it holds and the time in seconds, such as `connections.json.v1-1762300000`. It keeps the newest three copies of each file. What you delete from the history or the saved queries, and what the history's retention setting drops, is deleted from the copies too. To give the older release its files back: From f0b67b802a92d5039d9484627b1ad7e7c43b5aad Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:21:44 +0700 Subject: [PATCH 14/24] fix(core-storage): upgrade copies keep only what the user has not deleted --- core/crates/storage/src/connections/mod.rs | 3 + core/crates/storage/src/query_history/mod.rs | 160 ++++++++---------- core/crates/storage/src/saved_queries/mod.rs | 20 +-- core/crates/storage/src/upgrade_copies.rs | 64 +++++++ .../0011-copies-before-a-format-upgrade.md | 2 +- .../src/persistence/column_layout_store.rs | 4 + .../app/src/persistence/column_width_store.rs | 4 + .../src/persistence/filter_settings_store.rs | 4 + .../crates/app/src/persistence/state_file.rs | 6 + .../app/src/services/connection_deletion.rs | 21 +++ linux/docs/storage.md | 18 +- linux/docs/troubleshooting.md | 2 +- 12 files changed, 187 insertions(+), 121 deletions(-) diff --git a/core/crates/storage/src/connections/mod.rs b/core/crates/storage/src/connections/mod.rs index 8631430902..cea6bea0b6 100644 --- a/core/crates/storage/src/connections/mod.rs +++ b/core/crates/storage/src/connections/mod.rs @@ -589,7 +589,10 @@ impl ConnectionStore { if document.connections.len() == before { return Ok(RemoveOutcome::NotPresent); } + let kept = document.connections.iter().map(|saved| saved.id.to_string()).collect(); self.commit(&mut state, document)?; + // The commit itself may have just copied the older file with the connection in it. + crate::upgrade_copies::trim_connection_copies_blocking(&self.inner.path, &kept)?; Ok(RemoveOutcome::Removed) } diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index f013dd2faa..69a90e1afc 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -52,7 +52,7 @@ const ENTRY_COLUMNS: &str = "h.id, h.query, h.driver_id, h.connection_id, h.conn h.duration_ms, h.rows_affected, h.success, h.cancelled, h.pinned, h.error"; /// A value for a placeholder of a statement built from parts. -pub(crate) enum Bind { +enum Bind { Text(String), Integer(i64), } @@ -264,17 +264,7 @@ impl QueryHistory { } } transaction.commit().await?; - let in_copies = ids - .chunks(IDS_PER_STATEMENT) - .map(|chunk| { - let placeholders = vec!["?"; chunk.len()].join(","); - ( - format!("DELETE FROM history WHERE id IN ({placeholders})"), - chunk.iter().map(|id| Bind::Integer(*id)).collect(), - ) - }) - .collect(); - delete_in_upgrade_copies(&self.path, "history", in_copies).await?; + trim_upgrade_copies(&self.path).await?; deleted.sort_by_key(|entry| entry.id); Ok(deleted) } @@ -322,12 +312,7 @@ impl QueryHistory { .execute(&self.pool) .await?; } - delete_in_upgrade_copies( - &self.path, - "history", - vec![("DELETE FROM history".to_owned(), Vec::new())], - ) - .await?; + trim_upgrade_copies(&self.path).await?; Ok(affected as usize) } @@ -347,16 +332,6 @@ impl QueryHistory { .execute(&self.pool) .await? .rows_affected(); - // A copy keeps no query past the window the user chose either. - delete_in_upgrade_copies( - &self.path, - "history", - vec![( - "DELETE FROM history WHERE pinned = 0 AND executed_at < ?".to_owned(), - vec![Bind::Integer(to_unix(cutoff))], - )], - ) - .await?; } report.over_cap = sqlx::query( @@ -376,33 +351,18 @@ impl QueryHistory { .execute(&self.pool) .await?; } + // Rows expired here, or unpinned since a copy was made, leave the copies too. + trim_upgrade_copies(&self.path).await?; Ok(report) } /// How many queries ran on a connection, pinned ones included. - // The most any one place holds, the copies an upgrade kept included, so queries that outlived the - // retention window only in a copy can still go with the connection. pub async fn count_for_connection(&self, connection: Uuid) -> Result { let count: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM history WHERE connection_id = ?") .bind(connection.to_string()) .fetch_one(&self.pool) .await?; - let mut most = u64::try_from(count).unwrap_or(0); - for copy in crate::upgrade_copies::copies_blocking(&self.path)? { - let mut database = SqliteConnectOptions::new() - .filename(©.path) - .read_only(true) - .disable_statement_logging() - .connect() - .await?; - let kept: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM history WHERE connection_id = ?") - .bind(connection.to_string()) - .fetch_one(&mut database) - .await?; - database.close().await?; - most = most.max(u64::try_from(kept).unwrap_or(0)); - } - Ok(most) + Ok(u64::try_from(count).unwrap_or(0)) } /// Drop every query run on a connection, pinned ones included, for a @@ -420,15 +380,7 @@ impl QueryHistory { .execute(&self.pool) .await?; } - delete_in_upgrade_copies( - &self.path, - "history", - vec![( - "DELETE FROM history WHERE connection_id = ?".to_owned(), - vec![Bind::Text(connection.to_string())], - )], - ) - .await?; + trim_upgrade_copies(&self.path).await?; Ok(removed) } @@ -483,49 +435,61 @@ impl QueryHistory { } } -// What the user deleted, or the retention window dropped, goes from the copies an upgrade kept too: -// each full-text index is optimized, so no deleted word stays in its blocks, and VACUUM leaves none in -// free pages. A copy without `table` is left alone. -pub(crate) async fn delete_in_upgrade_copies( - path: &Path, - table: &str, - deletes: Vec<(String, Vec)>, -) -> Result<(), StorageError> { +// A copy an upgrade kept never holds a history entry or saved query the database no longer has, so +// what the user deleted, or the retention window dropped, is gone from it too. One transaction with +// secure_delete, so the words leave the full-text blocks and freed pages at once, even after a crash. +pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> { crate::upgrade_copies::remove_partials_blocking(path)?; - for copy in crate::upgrade_copies::copies_blocking(path)? { + let copies = crate::upgrade_copies::copies_blocking(path)?; + if copies.is_empty() { + return Ok(()); + } + let live = path.to_str().ok_or_else(|| StorageError::Io { + path: path.to_owned(), + source: std::io::Error::new(std::io::ErrorKind::InvalidInput, "the path is not UTF-8"), + })?; + for copy in copies { let mut database = SqliteConnectOptions::new() .filename(©.path) .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .pragma("secure_delete", "ON") .disable_statement_logging() .connect() .await?; - let has_table: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM sqlite_master WHERE type = 'table' AND name = ?") - .bind(table) - .fetch_one(&mut database) + sqlx::query("ATTACH DATABASE ? AS live") + .bind(live) + .execute(&mut database) .await?; + let mut transaction = database.begin().await?; let mut removed = 0; - if has_table > 0 { - for (sql, binds) in &deletes { - let mut statement = sqlx::query(AssertSqlSafe(sql.clone())); - for bind in binds { - statement = match bind { - Bind::Text(text) => statement.bind(text.clone()), - Bind::Integer(value) => statement.bind(*value), - }; - } - removed += statement.execute(&mut database).await?.rows_affected(); + for table in ["history", "saved_queries"] { + let present: i64 = sqlx::query_scalar( + "SELECT (SELECT COUNT(*) FROM main.sqlite_master WHERE type = 'table' AND name = ?1) \ + * (SELECT COUNT(*) FROM live.sqlite_master WHERE type = 'table' AND name = ?1)", + ) + .bind(table) + .fetch_one(&mut *transaction) + .await?; + if present > 0 { + removed += sqlx::query(AssertSqlSafe(format!( + "DELETE FROM main.{table} WHERE id NOT IN (SELECT id FROM live.{table})" + ))) + .execute(&mut *transaction) + .await? + .rows_affected(); } } if removed > 0 { - for index in full_text_indexes(&mut database).await? { + for index in full_text_indexes(&mut transaction).await? { sqlx::query(AssertSqlSafe(format!( - "INSERT INTO \"{index}\"(\"{index}\") VALUES('optimize')" + "INSERT INTO main.\"{index}\"(\"{index}\") VALUES('optimize')" ))) - .execute(&mut database) + .execute(&mut *transaction) .await?; } - sqlx::query("VACUUM").execute(&mut database).await?; } + transaction.commit().await?; + sqlx::query("DETACH DATABASE live").execute(&mut database).await?; database.close().await?; } Ok(()) @@ -534,7 +498,7 @@ pub(crate) async fn delete_in_upgrade_copies( // A copy holds the schema of the build that wrote it, so its indexes are found rather than named. async fn full_text_indexes(database: &mut sqlx::SqliteConnection) -> Result, StorageError> { let names: Vec = sqlx::query_scalar( - "SELECT name FROM sqlite_master WHERE type = 'table' AND sql LIKE 'CREATE VIRTUAL TABLE%USING fts5%'", + "SELECT name FROM main.sqlite_master WHERE type = 'table' AND sql LIKE 'CREATE VIRTUAL TABLE%USING fts5%'", ) .fetch_all(&mut *database) .await?; @@ -1254,22 +1218,10 @@ mod tests { let root = tempfile::tempdir().expect("tempdir"); let (history, copy) = history_with_a_copy(&root).await; assert!(indexed(©, "zzz").await); - // As retention would: the live rows go and the copy keeps its own. - sqlx::query("DELETE FROM history WHERE connection_id = ?") - .bind(Uuid::max().to_string()) - .execute(&history.pool) - .await - .expect("the live rows expire"); - assert_eq!( - history.count_for_connection(Uuid::max()).await.expect("count"), - 1, - "the copy's queries were not counted" - ); history.delete_for_connection(Uuid::max()).await.expect("delete"); assert_eq!(copy_rows(©).await, 1); - assert_eq!(history.count_for_connection(Uuid::max()).await.expect("count"), 0); assert!( !indexed(©, "zzz").await, "a deleted query's words stayed in the copy's index" @@ -1277,6 +1229,26 @@ mod tests { assert!(indexed(©, "qqq").await, "another connection's query went"); } + // The copy took the row pinned; once unpinned it expires, and the copy follows the history. + #[tokio::test] + async fn a_row_pinned_in_a_copy_still_expires_once_unpinned() { + let root = tempfile::tempdir().expect("tempdir"); + let history = QueryHistory::open(&paths(&root)).await.expect("open"); + let mut old = entry("SELECT xxxx", Uuid::nil()); + old.executed_at = SystemTime::now() - Duration::from_secs(400 * 24 * 60 * 60); + let id = history.record(old).await.expect("record"); + history.set_pinned(id, true).await.expect("pin"); + let copy = crate::upgrade_copies::keep_database(&history.pool, &history.path, 1) + .await + .expect("a copy"); + + history.set_pinned(id, false).await.expect("unpin"); + history.prune(30).await.expect("prune"); + + assert_eq!(copy_rows(©).await, 0, "an expired query stayed in the copy"); + assert!(!indexed(©, "xxx").await); + } + #[tokio::test] async fn clearing_the_history_clears_the_copies_and_keeps_their_saved_queries() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/core/crates/storage/src/saved_queries/mod.rs b/core/crates/storage/src/saved_queries/mod.rs index 362b086e36..d3f9377d64 100644 --- a/core/crates/storage/src/saved_queries/mod.rs +++ b/core/crates/storage/src/saved_queries/mod.rs @@ -217,15 +217,7 @@ impl SavedQueries { .execute(&self.pool) .await? .rows_affected(); - crate::query_history::delete_in_upgrade_copies( - &self.path, - "saved_queries", - vec![( - "DELETE FROM saved_queries WHERE id = ?".to_owned(), - vec![crate::query_history::Bind::Integer(id)], - )], - ) - .await?; + crate::query_history::trim_upgrade_copies(&self.path).await?; match affected { 0 => Err(StorageError::NotFound), _ => Ok(()), @@ -241,15 +233,7 @@ impl SavedQueries { .execute(&self.pool) .await? .rows_affected(); - crate::query_history::delete_in_upgrade_copies( - &self.path, - "saved_queries", - vec![( - "DELETE FROM saved_queries WHERE connection_id = ?".to_owned(), - vec![crate::query_history::Bind::Text(connection.to_string())], - )], - ) - .await?; + crate::query_history::trim_upgrade_copies(&self.path).await?; Ok(affected as usize) } } diff --git a/core/crates/storage/src/upgrade_copies.rs b/core/crates/storage/src/upgrade_copies.rs index cf18c2a6c9..53a414d044 100644 --- a/core/crates/storage/src/upgrade_copies.rs +++ b/core/crates/storage/src/upgrade_copies.rs @@ -128,6 +128,47 @@ pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) Ok(copy) } +// A copy of the connection list keeps no connection the list no longer has, so a deleted one is gone +// from it too; a copy that cannot be read that way is deleted rather than kept with it. +pub fn trim_connection_copies_blocking( + path: &Path, + kept: &std::collections::HashSet, +) -> Result<(), StorageError> { + for copy in copies_blocking(path)? { + let bytes = match std::fs::read(©.path) { + Ok(bytes) => bytes, + Err(error) if error.kind() == ErrorKind::NotFound => continue, + Err(source) => { + return Err(StorageError::Io { + path: copy.path.clone(), + source, + }); + } + }; + let trimmed = serde_json::from_slice::(&bytes) + .ok() + .and_then(|mut document| { + let connections = document.get_mut("connections")?.as_array_mut()?; + let before = connections.len(); + connections.retain(|connection| { + connection + .get("id") + .and_then(serde_json::Value::as_str) + .is_some_and(|id| kept.contains(id)) + }); + Some((connections.len() != before).then_some(document)) + }); + match trimmed { + Some(None) => {} + Some(Some(document)) => { + crate::fs::write_private_blocking(©.path, &serde_json::to_vec_pretty(&document)?)?; + } + None => remove_if_present(©.path)?, + } + } + Ok(()) +} + // Oldest first. pub fn copies_blocking(path: &Path) -> Result, StorageError> { let Some(name) = path.file_name().map(|name| name.to_string_lossy().into_owned()) else { @@ -580,6 +621,29 @@ mod tests { ); } + #[test] + fn a_connection_list_copy_keeps_only_the_connections_still_saved() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("connections.json"); + let both = br#"{"version": 1, "connections": [{"id": "a", "host": "kept.example"}, {"id": "b", "host": "gone.example"}]}"#; + let kept_copy = keep_bytes_blocking(&file, 1, both, at(100)).expect("keep"); + let unreadable = keep_bytes_blocking(&file, 1, b"{ not json", at(200)).expect("keep"); + let kept: std::collections::HashSet = ["a".to_owned()].into_iter().collect(); + + trim_connection_copies_blocking(&file, &kept).expect("trim"); + + let text = std::fs::read_to_string(&kept_copy).expect("read"); + assert!( + text.contains("kept.example") && !text.contains("gone.example"), + "{text}" + ); + assert!(!unreadable.exists(), "a copy that could not be trimmed stayed"); + assert_eq!( + std::fs::metadata(&kept_copy).expect("metadata").permissions().mode() & 0o777, + 0o600 + ); + } + #[test] fn only_that_files_copies_are_counted_and_removed() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/core/docs/decisions/0011-copies-before-a-format-upgrade.md b/core/docs/decisions/0011-copies-before-a-format-upgrade.md index 3246f72544..adc7590906 100644 --- a/core/docs/decisions/0011-copies-before-a-format-upgrade.md +++ b/core/docs/decisions/0011-copies-before-a-format-upgrade.md @@ -18,7 +18,7 @@ Before the first write that replaces a file holding an older format, the storage - **Beside the file, like the copies a Reset moves aside.** `connections.json.v1-…` sits next to `connections.json.corrupt-…`, a restore is a rename in one folder, and a session's purge removes its copies with its file. - **Three per file, as Chromium keeps three user-data snapshots** taken at each major update for a rollback ([Keep data during version rollback](https://support.google.com/chrome/a/answer/9917429)). A copy whose bytes match the newest one is not made again. - **No copy, no write.** A failed copy fails the write, the file stays as it was, and the app words the error by the copy's path like any other failed write. Upgrading without the way back is what this decision removes. -- **What the user deletes is deleted from the copies.** Every deletion from the history or the saved queries, and the history's retention window, applies to each history copy as well, which then optimizes its full-text indexes and runs `VACUUM`, so a copy never outlives the user's choice about their SQL. +- **A copy never holds what the user deleted.** After any deletion from the history or the saved queries, and after each retention prune, a history copy keeps only the rows the database still has, deleted in one transaction with `secure_delete` and its full-text indexes optimized. A deleted connection leaves each copy of the connection list, and its column layout and filter copies are deleted. Mirroring each delete statement instead missed pins changed after the copy and left words behind after a crash between steps. ## Consequences diff --git a/linux/crates/app/src/persistence/column_layout_store.rs b/linux/crates/app/src/persistence/column_layout_store.rs index a3e8d9ed2a..7c30ebec17 100644 --- a/linux/crates/app/src/persistence/column_layout_store.rs +++ b/linux/crates/app/src/persistence/column_layout_store.rs @@ -27,6 +27,10 @@ impl ColumnLayoutStore { } } + pub fn path(&self) -> &std::path::Path { + self.file.path() + } + pub fn layout(&self, connection_id: Uuid, schema: Option<&str>, table: &str) -> ColumnLayout { self.read(connection_id, schema, table, |layout| { layout.cloned().unwrap_or_default() diff --git a/linux/crates/app/src/persistence/column_width_store.rs b/linux/crates/app/src/persistence/column_width_store.rs index b39af34626..19ccf0e1e5 100644 --- a/linux/crates/app/src/persistence/column_width_store.rs +++ b/linux/crates/app/src/persistence/column_width_store.rs @@ -24,6 +24,10 @@ impl ColumnWidthStore { &self.layouts } + pub fn path(&self) -> &std::path::Path { + self.layouts.path() + } + pub fn width(&self, connection_id: Uuid, schema: Option<&str>, table: &str, column: &str) -> Option { self.layouts.read(connection_id, schema, table, |layout| { layout.and_then(|layout| layout.widths.get(column).copied()) diff --git a/linux/crates/app/src/persistence/filter_settings_store.rs b/linux/crates/app/src/persistence/filter_settings_store.rs index 8a440292bd..69d50a9a88 100644 --- a/linux/crates/app/src/persistence/filter_settings_store.rs +++ b/linux/crates/app/src/persistence/filter_settings_store.rs @@ -21,6 +21,10 @@ impl FilterSettingsStore { } } + pub fn path(&self) -> &std::path::Path { + self.file.path() + } + pub fn filters(&self, connection_id: Uuid, schema: Option<&str>, table: &str) -> FilterSet { self.file.read(|document| { document diff --git a/linux/crates/app/src/persistence/state_file.rs b/linux/crates/app/src/persistence/state_file.rs index 0045a2b9dc..c22ab84591 100644 --- a/linux/crates/app/src/persistence/state_file.rs +++ b/linux/crates/app/src/persistence/state_file.rs @@ -12,6 +12,7 @@ use tablepro_storage::{DocumentProblem, DocumentProblemKind}; /// go through a latest-wins writer on a blocking thread, so a burst /// costs two writes rather than one per event. pub struct StateFile { + path: PathBuf, value: RefCell, persistence: Cell, writer: LatestWinsWriter, @@ -39,12 +40,17 @@ where let path_for_writes = path.clone(); let writer = LatestWinsWriter::spawn(tasks, move |document: &D| write(&path_for_writes, document)); Self { + path, value: RefCell::new(value), persistence: Cell::new(persistence), writer, } } + pub fn path(&self) -> &std::path::Path { + &self.path + } + pub fn read(&self, with: impl FnOnce(&D) -> R) -> R { with(&self.value.borrow()) } diff --git a/linux/crates/app/src/services/connection_deletion.rs b/linux/crates/app/src/services/connection_deletion.rs index 67521330c6..17d0ef13bf 100644 --- a/linux/crates/app/src/services/connection_deletion.rs +++ b/linux/crates/app/src/services/connection_deletion.rs @@ -10,6 +10,7 @@ use uuid::Uuid; use super::deletion_error::DeletionError; use super::deletion_report::{DeletionReport, PurgeFailure}; use crate::persistence::{ColumnWidthStore, FilterSettingsStore}; +use crate::workspace::WorkspaceSaveError; use crate::workspace::WorkspaceStore; /// What the user chose in the delete dialog. @@ -58,6 +59,26 @@ impl ConnectionDeletion { // cannot leave them on disk. self.column_widths.flush().await; self.filters.flush().await; + // Copies kept before an upgrade hold the connection's tables and filter values, and are not + // worth keeping without them. + let files = [self.column_widths.path().to_owned(), self.filters.path().to_owned()]; + let copies = self + .spawner + .spawn_blocking_task(move || { + files + .iter() + .try_for_each(|file| tablepro_storage::upgrade_copies::remove_all_blocking(file).map(drop)) + }) + .await; + if let Some(error) = match copies { + Ok(Ok(())) => None, + Ok(Err(error)) => Some(error.to_string()), + Err(failure) => Some(failure.to_string()), + } { + report + .purge_failures + .push(PurgeFailure::Workspace(WorkspaceSaveError::Purge(error))); + } if let Err(error) = self.workspaces.purge(&SessionKey::Saved(id)).await { report.purge_failures.push(PurgeFailure::Workspace(error)); } diff --git a/linux/docs/storage.md b/linux/docs/storage.md index e12171bb45..17ab87749d 100644 --- a/linux/docs/storage.md +++ b/linux/docs/storage.md @@ -251,11 +251,13 @@ can tell whether a re-pinned entry falls in the pages already read. - `delete_for_connection` removes one connection's entries, pinned ones included, for a connection deleted along with its history, then runs `optimize` as a prune does. -- Every deletion, `delete_returning`, `clear_all`, `delete_for_connection`, - the retention part of `prune` and the saved queries' `delete` and - `delete_for_connection`, deletes the same rows from each copy an upgrade - kept, optimizes its full-text indexes and runs `VACUUM`, so nothing of them - stays on disk (`delete_in_upgrade_copies`). +- After every deletion (`delete_returning`, `clear_all`, + `delete_for_connection`, `prune` and the saved queries' `delete` and + `delete_for_connection`), `trim_upgrade_copies` makes each copy an upgrade + kept hold no history entry or saved query the database no longer has. It + attaches the database and deletes the rest in one transaction with + `secure_delete` on, optimizing the copy's full-text indexes, so the words + leave its blocks and freed pages together. - `connections_with_history` lists each connection the history holds under the name of its latest run, so a renamed connection shows once. @@ -403,8 +405,10 @@ that replaces it with a newer format ([core ADR 0011](../../core/docs/decisions/ - A copy of a session's tabs names their editor drafts and holds none of their text. Drafts have no format version, so the older build reads them as they are now; a tab closed since the copy comes back without its text. -- `count_for_connection` counts the history's copies too, so the queries of a - deleted connection that only a copy still holds can go with it. +- `ConnectionStore::remove_blocking` takes the connection out of each copy of + `connections.json` too, and deletes a copy it cannot read that way. Deleting + a connection deletes the copies of the column layout and filter files, which + name its tables and filter values. - A copy that cannot be made fails the write or the open, and the file is left as it was. The error names the copy's path. - Reading never makes a copy. Nothing restores one on its own: diff --git a/linux/docs/troubleshooting.md b/linux/docs/troubleshooting.md index 30b861f75b..d0ea298f83 100644 --- a/linux/docs/troubleshooting.md +++ b/linux/docs/troubleshooting.md @@ -94,7 +94,7 @@ Installing the newer release again brings all of it back, unless it was reset. ### Going back to what the older release saved -Before a release writes a file in a newer format for the first time, it keeps a copy of the file as it was, in the same folder, named after the file, the format version it holds and the time in seconds, such as `connections.json.v1-1762300000`. It keeps the newest three copies of each file. What you delete from the history or the saved queries, and what the history's retention setting drops, is deleted from the copies too. +Before a release writes a file in a newer format for the first time, it keeps a copy of the file as it was, in the same folder, named after the file, the format version it holds and the time in seconds, such as `connections.json.v1-1762300000`. It keeps the newest three copies of each file. A copy never keeps what you deleted since: a connection, query history or saved queries you delete, and history the retention setting drops, leave the copies too. To give the older release its files back: From 9107bc0a6dcf96419eac625a8c3733b4b4f41741 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:28:21 +0700 Subject: [PATCH 15/24] fix(core-storage): scrub a new history copy's index and trim connection copies before each write --- core/crates/storage/src/connections/mod.rs | 11 ++-- core/crates/storage/src/query_history/mod.rs | 61 +++++++++++++++---- core/crates/storage/src/upgrade_copies.rs | 57 +++++++++++++++++ .../0011-copies-before-a-format-upgrade.md | 2 +- linux/docs/storage.md | 9 ++- 5 files changed, 120 insertions(+), 20 deletions(-) diff --git a/core/crates/storage/src/connections/mod.rs b/core/crates/storage/src/connections/mod.rs index cea6bea0b6..dd38d5bec0 100644 --- a/core/crates/storage/src/connections/mod.rs +++ b/core/crates/storage/src/connections/mod.rs @@ -589,10 +589,7 @@ impl ConnectionStore { if document.connections.len() == before { return Ok(RemoveOutcome::NotPresent); } - let kept = document.connections.iter().map(|saved| saved.id.to_string()).collect(); self.commit(&mut state, document)?; - // The commit itself may have just copied the older file with the connection in it. - crate::upgrade_copies::trim_connection_copies_blocking(&self.inner.path, &kept)?; Ok(RemoveOutcome::Removed) } @@ -870,7 +867,13 @@ impl ConnectionStore { fn commit(&self, state: &mut StoreState, mut document: ConnectionsDocument) -> Result<(), StorageError> { // A connection saved with a tag deleted meanwhile drops it here. document.tidy(); - crate::document::write_document_blocking(&self.inner.path, &document)?; + let bytes = crate::document::encode_document(&document)?; + crate::upgrade_copies::keep_before_write_blocking::(&self.inner.path)?; + // Before the list itself, so a deleted connection is never gone from the list but kept in a + // copy, and a failed trim leaves both as they were for the next write to try again. + let kept = document.connections.iter().map(|saved| saved.id.to_string()).collect(); + crate::upgrade_copies::trim_connection_copies_blocking(&self.inner.path, &kept)?; + crate::fs::write_private_blocking(&self.inner.path, &bytes)?; *state = StoreState::Ready(document); self.publish(state); Ok(()) diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index 69a90e1afc..9b61eab5a3 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -461,7 +461,6 @@ pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> .execute(&mut database) .await?; let mut transaction = database.begin().await?; - let mut removed = 0; for table in ["history", "saved_queries"] { let present: i64 = sqlx::query_scalar( "SELECT (SELECT COUNT(*) FROM main.sqlite_master WHERE type = 'table' AND name = ?1) \ @@ -471,23 +470,15 @@ pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> .fetch_one(&mut *transaction) .await?; if present > 0 { - removed += sqlx::query(AssertSqlSafe(format!( - "DELETE FROM main.{table} WHERE id NOT IN (SELECT id FROM live.{table})" - ))) - .execute(&mut *transaction) - .await? - .rows_affected(); - } - } - if removed > 0 { - for index in full_text_indexes(&mut transaction).await? { sqlx::query(AssertSqlSafe(format!( - "INSERT INTO main.\"{index}\"(\"{index}\") VALUES('optimize')" + "DELETE FROM main.{table} WHERE id NOT IN (SELECT id FROM live.{table})" ))) .execute(&mut *transaction) .await?; } } + // Every time, which costs little on an index already merged. + optimize_full_text_indexes(&mut transaction).await?; transaction.commit().await?; sqlx::query("DETACH DATABASE live").execute(&mut database).await?; database.close().await?; @@ -495,6 +486,32 @@ pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> Ok(()) } +// A copy fresh from VACUUM INTO still holds, in its full-text blocks, the words of entries deleted +// before it was made. +pub(crate) async fn scrub_copy(copy: &Path) -> Result<(), StorageError> { + let mut database = SqliteConnectOptions::new() + .filename(copy) + .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .pragma("secure_delete", "ON") + .disable_statement_logging() + .connect() + .await?; + optimize_full_text_indexes(&mut database).await?; + database.close().await?; + Ok(()) +} + +async fn optimize_full_text_indexes(database: &mut sqlx::SqliteConnection) -> Result<(), StorageError> { + for index in full_text_indexes(database).await? { + sqlx::query(AssertSqlSafe(format!( + "INSERT INTO main.\"{index}\"(\"{index}\") VALUES('optimize')" + ))) + .execute(&mut *database) + .await?; + } + Ok(()) +} + // A copy holds the schema of the build that wrote it, so its indexes are found rather than named. async fn full_text_indexes(database: &mut sqlx::SqliteConnection) -> Result, StorageError> { let names: Vec = sqlx::query_scalar( @@ -1347,4 +1364,24 @@ mod tests { assert_eq!(names, ["second"], "a deleted saved query stayed in the copy"); assert!(all(&history).await.iter().any(|entry| entry.id == kept)); } + + // VACUUM INTO copies the index blocks as they are, words of entries already deleted included. + #[tokio::test] + async fn a_copy_holds_no_word_of_an_entry_deleted_before_it_was_made() { + let root = tempfile::tempdir().expect("tempdir"); + let history = QueryHistory::open(&paths(&root)).await.expect("open"); + history.record(entry("SELECT qqqq", Uuid::nil())).await.expect("record"); + let deleted = history.record(entry("SELECT zzzz", Uuid::nil())).await.expect("record"); + history.delete_returning(&[deleted]).await.expect("delete"); + + let copy = crate::upgrade_copies::keep_database(&history.pool, &history.path, 1) + .await + .expect("a copy"); + + assert!( + !indexed(©, "zzz").await, + "a deleted entry's words came along into the copy" + ); + assert!(indexed(©, "qqq").await); + } } diff --git a/core/crates/storage/src/upgrade_copies.rs b/core/crates/storage/src/upgrade_copies.rs index 53a414d044..0b0496c421 100644 --- a/core/crates/storage/src/upgrade_copies.rs +++ b/core/crates/storage/src/upgrade_copies.rs @@ -105,6 +105,10 @@ pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) source: database_io_error(&error), }); } + if let Err(error) = crate::query_history::scrub_copy(&partial).await { + let _ = std::fs::remove_file(&partial); + return Err(error); + } // Renamed into place only once on disk, so a copy cut short by a // power cut never carries a name that says it is whole. let write_error = |source| StorageError::Write { @@ -644,6 +648,59 @@ mod tests { ); } + fn saved(name: &str) -> crate::SavedConnection { + crate::SavedConnection { + id: uuid::Uuid::new_v4(), + name: name.to_owned(), + driver_id: "postgres".to_owned(), + host: format!("{name}.example"), + port: 5432, + database: "app".to_owned(), + username: "admin".to_owned(), + tls: tablepro_core::TlsConfig::disabled(), + safe_mode: tablepro_core::SafeModeLevel::Off, + external_access: Default::default(), + ai_policy: Default::default(), + query_timeout_secs: None, + auth_mode: Default::default(), + ssh: None, + last_opened_at: None, + color: None, + group_id: None, + order: 0, + tags: Vec::new(), + icon: None, + options: Default::default(), + } + } + + // The copy here is written by hand, so the list needs no older format to test with. + #[test] + fn deleting_a_connection_takes_it_out_of_the_copies_before_the_list_is_written() { + let root = tempfile::tempdir().expect("tempdir"); + let paths = crate::StoragePaths::under(root.path(), "tablepro", "app.tablepro.TablePro"); + let store = crate::ConnectionStore::new(&paths); + let (kept, deleted) = (saved("kept"), saved("deleted")); + store.upsert_blocking(kept.clone()).expect("save"); + store.upsert_blocking(deleted.clone()).expect("save"); + let copy = keep_bytes_blocking( + &paths.connections_file(), + 1, + &std::fs::read(paths.connections_file()).expect("the list"), + at(100), + ) + .expect("a copy"); + + store.remove_blocking(deleted.id).expect("remove"); + + let text = std::fs::read_to_string(©).expect("read"); + assert!(text.contains("kept.example"), "{text}"); + assert!( + !text.contains("deleted.example"), + "the deleted connection stayed in the copy" + ); + } + #[test] fn only_that_files_copies_are_counted_and_removed() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/core/docs/decisions/0011-copies-before-a-format-upgrade.md b/core/docs/decisions/0011-copies-before-a-format-upgrade.md index adc7590906..5fa91c3021 100644 --- a/core/docs/decisions/0011-copies-before-a-format-upgrade.md +++ b/core/docs/decisions/0011-copies-before-a-format-upgrade.md @@ -18,7 +18,7 @@ Before the first write that replaces a file holding an older format, the storage - **Beside the file, like the copies a Reset moves aside.** `connections.json.v1-…` sits next to `connections.json.corrupt-…`, a restore is a rename in one folder, and a session's purge removes its copies with its file. - **Three per file, as Chromium keeps three user-data snapshots** taken at each major update for a rollback ([Keep data during version rollback](https://support.google.com/chrome/a/answer/9917429)). A copy whose bytes match the newest one is not made again. - **No copy, no write.** A failed copy fails the write, the file stays as it was, and the app words the error by the copy's path like any other failed write. Upgrading without the way back is what this decision removes. -- **A copy never holds what the user deleted.** After any deletion from the history or the saved queries, and after each retention prune, a history copy keeps only the rows the database still has, deleted in one transaction with `secure_delete` and its full-text indexes optimized. A deleted connection leaves each copy of the connection list, and its column layout and filter copies are deleted. Mirroring each delete statement instead missed pins changed after the copy and left words behind after a crash between steps. +- **A copy never holds what the user deleted.** After any deletion from the history or the saved queries, and after each retention prune, a history copy keeps only the rows the database still has, deleted in one transaction with `secure_delete` and its full-text indexes optimized. A new history copy has its full-text indexes optimized before it gets its name, since `VACUUM INTO` keeps the words of entries deleted earlier. Every write of the connection list first takes the connections it no longer has out of each copy, so a failed trim fails the write and the next one retries; a deleted connection's column layout and filter copies are deleted. Mirroring each delete statement instead missed pins changed after the copy and left words behind after a crash between steps. ## Consequences diff --git a/linux/docs/storage.md b/linux/docs/storage.md index 17ab87749d..474f04fa7a 100644 --- a/linux/docs/storage.md +++ b/linux/docs/storage.md @@ -399,14 +399,17 @@ that replaces it with a newer format ([core ADR 0011](../../core/docs/decisions/ bytes as read, written 0600 by `write_private_blocking`; a copy whose bytes match the newest one is not made again. The history's is `VACUUM INTO` through the open connection, so it holds what the WAL holds too, into a 0600 - `.partial` file that is synced and renamed into place. + `.partial` file. Its full-text indexes are then optimized with `secure_delete` + on, since `VACUUM INTO` copies the words of entries deleted before, and only + then is it synced and renamed into place. - Each file keeps its newest 3 copies (`KEPT_PER_FILE`); a new one deletes the oldest past that. - A copy of a session's tabs names their editor drafts and holds none of their text. Drafts have no format version, so the older build reads them as they are now; a tab closed since the copy comes back without its text. -- `ConnectionStore::remove_blocking` takes the connection out of each copy of - `connections.json` too, and deletes a copy it cannot read that way. Deleting +- Every write of the connection list first takes the connections it no longer + has out of each copy of `connections.json`, and deletes a copy it cannot + read that way; a failed trim fails the write, so the next one tries again. Deleting a connection deletes the copies of the column layout and filter files, which name its tables and filter values. - A copy that cannot be made fails the write or the open, and the file is From a9149405f9bc3db33602858ea050d10221a56da2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:33:20 +0700 Subject: [PATCH 16/24] fix(core-storage): remove the SQLite journals of unfinished and deleted copies --- core/crates/storage/src/upgrade_copies.rs | 35 +++++++++++++++++++---- 1 file changed, 30 insertions(+), 5 deletions(-) diff --git a/core/crates/storage/src/upgrade_copies.rs b/core/crates/storage/src/upgrade_copies.rs index 0b0496c421..c4dfe16f87 100644 --- a/core/crates/storage/src/upgrade_copies.rs +++ b/core/crates/storage/src/upgrade_copies.rs @@ -14,6 +14,9 @@ pub const KEPT_PER_FILE: usize = 3; const MAX_NAME_ATTEMPTS: u32 = 1_000; +// What SQLite keeps beside a database it writes: a crash leaves the pages it was replacing in them. +const SIDECARS: [&str; 3] = ["-journal", "-wal", "-shm"]; + // Named `.v-`, with `-` after it for a second copy in one second. #[derive(Debug, Clone, PartialEq, Eq)] pub struct UpgradeCopy { @@ -99,14 +102,14 @@ pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) source: std::io::Error::new(ErrorKind::InvalidInput, "the path is not UTF-8"), })?; if let Err(error) = sqlx::query("VACUUM INTO ?").bind(target).execute(pool).await { - let _ = std::fs::remove_file(&partial); + let _ = remove_with_sidecars(&partial); return Err(StorageError::Write { path: copy, source: database_io_error(&error), }); } if let Err(error) = crate::query_history::scrub_copy(&partial).await { - let _ = std::fs::remove_file(&partial); + let _ = remove_with_sidecars(&partial); return Err(error); } // Renamed into place only once on disk, so a copy cut short by a @@ -122,7 +125,7 @@ pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) .and_then(|file| file.sync_all()) .and_then(|()| std::fs::rename(&partial, ©)); if let Err(error) = landed { - let _ = std::fs::remove_file(&partial); + let _ = remove_with_sidecars(&partial); return Err(write_error(error)); } if let Some(directory) = copy.parent() { @@ -207,7 +210,7 @@ pub fn remove_all_blocking(path: &Path) -> Result { remove_partials_blocking(path)?; let copies = copies_blocking(path)?; for copy in &copies { - remove_if_present(©.path)?; + remove_with_sidecars(©.path)?; } Ok(copies.len()) } @@ -236,7 +239,11 @@ pub fn remove_partials_blocking(path: &Path) -> Result<(), StorageError> { source, })?; let found = entry.file_name().to_string_lossy().into_owned(); - if found.starts_with(&prefix) && found.ends_with(".partial") { + let unfinished = found.ends_with(".partial") + || SIDECARS + .iter() + .any(|sidecar| found.ends_with(&format!(".partial{sidecar}"))); + if found.starts_with(&prefix) && unfinished { remove_if_present(&entry.path())?; } } @@ -318,6 +325,16 @@ fn parent_of(path: &Path) -> &Path { } } +fn remove_with_sidecars(path: &Path) -> Result<(), StorageError> { + remove_if_present(path)?; + for sidecar in SIDECARS { + let mut name = path.as_os_str().to_owned(); + name.push(sidecar); + remove_if_present(Path::new(&name))?; + } + Ok(()) +} + fn remove_if_present(path: &Path) -> Result<(), StorageError> { match std::fs::remove_file(path) { Err(error) if error.kind() != ErrorKind::NotFound => Err(StorageError::Io { @@ -501,6 +518,8 @@ mod tests { let path = root.path().join("items.db"); let abandoned = root.path().join(".items.db.v1-100.partial"); std::fs::write(&abandoned, b"half a copy").expect("seed"); + let journal = root.path().join(".items.db.v1-100.partial-journal"); + std::fs::write(&journal, b"pages a scrub was replacing").expect("seed"); let unrelated = root.path().join(".other.db.v1-100.partial"); std::fs::write(&unrelated, b"another file's").expect("seed"); let pool = database(&path).await; @@ -509,10 +528,16 @@ mod tests { keep_database(&pool, &path, 1).await.expect("a copy"); assert!(!abandoned.exists(), "the unfinished copy stayed"); + assert!(!journal.exists(), "the unfinished copy's journal stayed"); assert!(unrelated.exists(), "another file's copy went"); std::fs::write(&abandoned, b"half a copy").expect("seed"); + let copy = copies_blocking(&path).expect("copies").pop().expect("the copy").path; + let mut sidecar = copy.as_os_str().to_owned(); + sidecar.push("-journal"); + std::fs::write(&sidecar, b"a trim's journal").expect("seed"); remove_all_blocking(&path).expect("remove"); assert!(!abandoned.exists(), "a purge left the unfinished copy"); + assert!(!Path::new(&sidecar).exists(), "a purge left a copy's journal"); } #[tokio::test] From 9c508af592b8237c68811af90b1cc7932229cdb5 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:39:03 +0700 Subject: [PATCH 17/24] fix(core-storage): sweep what a crash leaves of a copy, journals before databases --- core/crates/storage/src/query_history/mod.rs | 2 +- core/crates/storage/src/upgrade_copies.rs | 60 ++++++++++++++++---- 2 files changed, 49 insertions(+), 13 deletions(-) diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index 9b61eab5a3..88de0f708c 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -439,7 +439,7 @@ impl QueryHistory { // what the user deleted, or the retention window dropped, is gone from it too. One transaction with // secure_delete, so the words leave the full-text blocks and freed pages at once, even after a crash. pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> { - crate::upgrade_copies::remove_partials_blocking(path)?; + crate::upgrade_copies::remove_leftovers_blocking(path)?; let copies = crate::upgrade_copies::copies_blocking(path)?; if copies.is_empty() { return Ok(()); diff --git a/core/crates/storage/src/upgrade_copies.rs b/core/crates/storage/src/upgrade_copies.rs index c4dfe16f87..ae25ed3ae7 100644 --- a/core/crates/storage/src/upgrade_copies.rs +++ b/core/crates/storage/src/upgrade_copies.rs @@ -49,6 +49,7 @@ pub fn keep_before_write_blocking(path: &Path) -> Result Result { + remove_leftovers_blocking(path)?; if let Some(newest) = copies_blocking(path)?.pop() && std::fs::read(&newest.path).is_ok_and(|kept| kept == bytes) { @@ -91,7 +92,7 @@ pub(crate) async fn pending_upgrade(pool: &SqlitePool, migrator: &Migrator) -> R // Through the connection, since a copy of the file alone leaves out what the WAL still holds. pub(crate) async fn keep_database(pool: &SqlitePool, path: &Path, version: u64) -> Result { // One a crash left behind holds the history too, and nothing else would ever delete it. - remove_partials_blocking(path)?; + remove_leftovers_blocking(path)?; let copy = vacant_name(path, version, SystemTime::now())?; let partial = partial_name(©); // VACUUM INTO refuses a file that is not empty, and creates a missing @@ -141,6 +142,7 @@ pub fn trim_connection_copies_blocking( path: &Path, kept: &std::collections::HashSet, ) -> Result<(), StorageError> { + remove_leftovers_blocking(path)?; for copy in copies_blocking(path)? { let bytes = match std::fs::read(©.path) { Ok(bytes) => bytes, @@ -207,7 +209,7 @@ pub fn copies_blocking(path: &Path) -> Result, StorageError> { } pub fn remove_all_blocking(path: &Path) -> Result { - remove_partials_blocking(path)?; + remove_leftovers_blocking(path)?; let copies = copies_blocking(path)?; for copy in &copies { remove_with_sidecars(©.path)?; @@ -216,8 +218,9 @@ pub fn remove_all_blocking(path: &Path) -> Result { } // The copy just made stays even when a clock set back dates it before the others. -// Copies of a database that never finished, as `..v-.partial`. -pub fn remove_partials_blocking(path: &Path) -> Result<(), StorageError> { +// What a crash leaves of a copy: its staging (`..v…`, an unfinished database, a temporary file +// and their journals) and the journals of a copy that is gone. They hold what the copy held. +pub fn remove_leftovers_blocking(path: &Path) -> Result<(), StorageError> { let Some(name) = path.file_name().map(|name| name.to_string_lossy().into_owned()) else { return Ok(()); }; @@ -232,18 +235,27 @@ pub fn remove_partials_blocking(path: &Path) -> Result<(), StorageError> { }); } }; - let prefix = format!(".{name}.v"); + let (staging, copy) = (format!(".{name}.v"), format!("{name}.v")); for entry in entries { let entry = entry.map_err(|source| StorageError::Io { path: parent.to_owned(), source, })?; let found = entry.file_name().to_string_lossy().into_owned(); - let unfinished = found.ends_with(".partial") - || SIDECARS + let orphan_journal = found.starts_with(©) + && SIDECARS .iter() - .any(|sidecar| found.ends_with(&format!(".partial{sidecar}"))); - if found.starts_with(&prefix) && unfinished { + .find_map(|sidecar| found.strip_suffix(sidecar)) + .is_some_and(|base| !parent.join(base).exists()); + // `-` after the dot, which a temporary file of the file itself never has. + let staged = found.strip_prefix(&staging).is_some_and(|rest| { + let stamp: Vec<&str> = rest.split('.').next().unwrap_or_default().split('-').collect(); + stamp.len() >= 2 + && stamp + .iter() + .all(|part| !part.is_empty() && part.bytes().all(|byte| byte.is_ascii_digit())) + }); + if staged || orphan_journal { remove_if_present(&entry.path())?; } } @@ -257,7 +269,7 @@ fn prune_blocking(path: &Path, made: &Path) -> Result<(), StorageError> { .collect(); let surplus = older.len().saturating_sub(KEPT_PER_FILE - 1); for copy in older.iter().take(surplus) { - remove_if_present(©.path)?; + remove_with_sidecars(©.path)?; } Ok(()) } @@ -325,14 +337,14 @@ fn parent_of(path: &Path) -> &Path { } } +// Journals first, so one stopped halfway leaves the database, which the next cleanup still finds. fn remove_with_sidecars(path: &Path) -> Result<(), StorageError> { - remove_if_present(path)?; for sidecar in SIDECARS { let mut name = path.as_os_str().to_owned(); name.push(sidecar); remove_if_present(Path::new(&name))?; } - Ok(()) + remove_if_present(path) } fn remove_if_present(path: &Path) -> Result<(), StorageError> { @@ -726,6 +738,30 @@ mod tests { ); } + #[test] + fn a_trim_removes_what_a_crash_left_of_a_copy() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("connections.json"); + let staged = root.path().join(".connections.json.v1-100.a1b2c3.tmp"); + let own_write = root.path().join(".connections.json.vQ3x9k.tmp"); + std::fs::write(&own_write, b"the list being written").expect("seed"); + std::fs::write(&staged, br#"{"connections": [{"id": "gone"}]}"#).expect("seed"); + let orphan = root.path().join("connections.json.v1-50-journal"); + std::fs::write(&orphan, b"pages").expect("seed"); + let kept = keep_bytes_blocking(&file, 1, br#"{"connections": []}"#, at(100)).expect("keep"); + let mut journal = kept.as_os_str().to_owned(); + journal.push("-journal"); + std::fs::write(&journal, b"a live copy's journal").expect("seed"); + std::fs::write(&staged, b"again").expect("seed"); + + trim_connection_copies_blocking(&file, &std::collections::HashSet::new()).expect("trim"); + + assert!(!staged.exists(), "a staged copy stayed"); + assert!(own_write.exists(), "the list's own temporary file went"); + assert!(!orphan.exists(), "the journal of a copy that is gone stayed"); + assert!(Path::new(&journal).exists(), "a journal beside its copy went"); + } + #[test] fn only_that_files_copies_are_counted_and_removed() { let root = tempfile::tempdir().expect("tempdir"); From 5b529f98acab05e49fa53d6938ebd44aee1e8a17 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:52:29 +0700 Subject: [PATCH 18/24] fix(linux): keep a script's location as handed over, and drop a history copy that cannot be trimmed --- core/crates/storage/src/query_history/mod.rs | 83 ++++++++++++------- core/crates/storage/src/upgrade_copies.rs | 4 + linux/ARCHITECTURE.md | 2 +- .../app/src/services/file_resolution.rs | 35 ++------ 4 files changed, 63 insertions(+), 61 deletions(-) diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index 88de0f708c..22604cf27e 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -449,40 +449,49 @@ pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> source: std::io::Error::new(std::io::ErrorKind::InvalidInput, "the path is not UTF-8"), })?; for copy in copies { - let mut database = SqliteConnectOptions::new() - .filename(©.path) - .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) - .pragma("secure_delete", "ON") - .disable_statement_logging() - .connect() - .await?; - sqlx::query("ATTACH DATABASE ? AS live") - .bind(live) - .execute(&mut database) - .await?; - let mut transaction = database.begin().await?; - for table in ["history", "saved_queries"] { - let present: i64 = sqlx::query_scalar( - "SELECT (SELECT COUNT(*) FROM main.sqlite_master WHERE type = 'table' AND name = ?1) \ - * (SELECT COUNT(*) FROM live.sqlite_master WHERE type = 'table' AND name = ?1)", - ) - .bind(table) - .fetch_one(&mut *transaction) + // A copy that cannot be trimmed, being damaged or unreadable, goes rather than keep what the + // user deleted. + if trim_copy(©.path, live).await.is_err() { + crate::upgrade_copies::remove_copy_blocking(©.path)?; + } + } + Ok(()) +} + +async fn trim_copy(copy: &Path, live: &str) -> Result<(), StorageError> { + let mut database = SqliteConnectOptions::new() + .filename(copy) + .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) + .pragma("secure_delete", "ON") + .disable_statement_logging() + .connect() + .await?; + sqlx::query("ATTACH DATABASE ? AS live") + .bind(live) + .execute(&mut database) + .await?; + let mut transaction = database.begin().await?; + for table in ["history", "saved_queries"] { + let present: i64 = sqlx::query_scalar( + "SELECT (SELECT COUNT(*) FROM main.sqlite_master WHERE type = 'table' AND name = ?1) \ + * (SELECT COUNT(*) FROM live.sqlite_master WHERE type = 'table' AND name = ?1)", + ) + .bind(table) + .fetch_one(&mut *transaction) + .await?; + if present > 0 { + sqlx::query(AssertSqlSafe(format!( + "DELETE FROM main.{table} WHERE id NOT IN (SELECT id FROM live.{table})" + ))) + .execute(&mut *transaction) .await?; - if present > 0 { - sqlx::query(AssertSqlSafe(format!( - "DELETE FROM main.{table} WHERE id NOT IN (SELECT id FROM live.{table})" - ))) - .execute(&mut *transaction) - .await?; - } } - // Every time, which costs little on an index already merged. - optimize_full_text_indexes(&mut transaction).await?; - transaction.commit().await?; - sqlx::query("DETACH DATABASE live").execute(&mut database).await?; - database.close().await?; } + // Every time, which costs little on an index already merged. + optimize_full_text_indexes(&mut transaction).await?; + transaction.commit().await?; + sqlx::query("DETACH DATABASE live").execute(&mut database).await?; + database.close().await?; Ok(()) } @@ -1384,4 +1393,16 @@ mod tests { ); assert!(indexed(©, "qqq").await); } + + #[tokio::test] + async fn a_copy_that_cannot_be_trimmed_is_removed() { + let root = tempfile::tempdir().expect("tempdir"); + let history = QueryHistory::open(&paths(&root)).await.expect("open"); + let damaged = root.path().join("state/tablepro/history.db.v1-100"); + std::fs::write(&damaged, b"not a database").expect("seed"); + + history.clear_all().await.expect("clear"); + + assert!(!damaged.exists(), "a copy that could not be trimmed stayed"); + } } diff --git a/core/crates/storage/src/upgrade_copies.rs b/core/crates/storage/src/upgrade_copies.rs index ae25ed3ae7..1e67f43567 100644 --- a/core/crates/storage/src/upgrade_copies.rs +++ b/core/crates/storage/src/upgrade_copies.rs @@ -337,6 +337,10 @@ fn parent_of(path: &Path) -> &Path { } } +pub(crate) fn remove_copy_blocking(path: &Path) -> Result<(), StorageError> { + remove_with_sidecars(path) +} + // Journals first, so one stopped halfway leaves the database, which the next cleanup still finds. fn remove_with_sidecars(path: &Path) -> Result<(), StorageError> { for sidecar in SIDECARS { diff --git a/linux/ARCHITECTURE.md b/linux/ARCHITECTURE.md index f2801d1f59..2a9c488e9d 100644 --- a/linux/ARCHITECTURE.md +++ b/linux/ARCHITECTURE.md @@ -75,7 +75,7 @@ One second runtime remains. Relm4 runs a component's commands (`sender.command`, A quit freezes the record before it asks the first window, so each window it closes stays on it, and writes it before the process may end. An approved quit then empties the list in memory; a cancelled one keeps only the windows still showing or connecting their session. Shutdown freezes it too: windows that go with the process are not the user closing them. An approved quit can leave the process running while a session closes, and an activate that reaches it then restores the record as a new launch would. -Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`); one GIO reaches without a local path, such as over SFTP, is a script by its name. A tab already bound to the same location comes forward; the lookup goes by location, so the GTK thread reads no file system. Otherwise the script goes to the most recently focused window, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no relational session in front, or while any window is still restoring, connecting or reading its saved tabs, where a tab bound to the file may yet appear, a picker asks: the script waits behind its banner until a relational session joins that window. A session that runs no SQL hands the question to a new picker, and one chosen while it is open in another window takes the scripts there. +Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`); one GIO reaches without a local path, such as over SFTP, is a script by its name. A tab already bound to the same location comes forward. The location is kept as handed over, as Open SQL File keeps it, and the lookup reads no file system on the GTK thread, so a tab bound through another path, such as a link's target, is not found. Otherwise the script goes to the most recently focused window, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no relational session in front, or while any window is still restoring, connecting or reading its saved tabs, where a tab bound to the file may yet appear, a picker asks: the script waits behind its banner until a relational session joins that window. A session that runs no SQL hands the question to a new picker, and one chosen while it is open in another window takes the scripts there. ## Windows and sessions diff --git a/linux/crates/app/src/services/file_resolution.rs b/linux/crates/app/src/services/file_resolution.rs index ec3390d7a2..ec852d1834 100644 --- a/linux/crates/app/src/services/file_resolution.rs +++ b/linux/crates/app/src/services/file_resolution.rs @@ -69,12 +69,9 @@ pub fn resolve_file_blocking( target: Ok(FileTarget::Script), }; } - if let Some(script) = path.as_deref().filter(|path| is_script_blocking(path, drivers)) { - // One file reached through a link and through its target opens one tab, as a database opens one session. - let uri = std::fs::canonicalize(script) - .ok() - .and_then(|canonical| glib::filename_to_uri(&canonical, None).ok()) - .map_or(uri, |canonical| canonical.to_string()); + // Kept as handed over, as Open SQL File binds a tab to the location it was given, so the same + // location finds the same tab whichever way it came. + if path.as_deref().is_some_and(|path| is_script_blocking(path, drivers)) { return ResolvedFile { uri, target: Ok(FileTarget::Script), @@ -431,7 +428,7 @@ mod tests { } #[test] - fn a_script_through_a_link_resolves_to_its_target() { + fn a_script_keeps_the_location_it_was_handed() { let root = tempfile::tempdir().expect("tempdir"); let target = root.path().join("report.sql"); std::fs::write(&target, b"SELECT 1;").expect("seed"); @@ -442,35 +439,15 @@ mod tests { "tablepro-test", "app.tablepro.Test", )); + let uri = format!("file://{}", link.display()); let resolved = resolve_file_blocking( Some(link.clone()), - format!("file://{}", link.display()), + uri.clone(), &store, &drivers(&FakeDriver::FILE_CAPABILITIES), ); - assert!( - matches!(resolved.target, Ok(FileTarget::Script)), - "{:?}", - resolved.target - ); - let canonical = std::fs::canonicalize(&target).expect("canonical"); - assert_eq!(gio::File::for_uri(&resolved.uri).path(), Some(canonical)); - } - - #[test] - fn a_remote_sql_file_with_no_local_path_is_a_script() { - let root = tempfile::tempdir().expect("tempdir"); - let store = ConnectionStore::new(&tablepro_storage::StoragePaths::under( - root.path(), - "tablepro-test", - "app.tablepro.Test", - )); - let uri = "sftp://db.example/home/ada/report.SQL".to_owned(); - - let resolved = resolve_file_blocking(None, uri.clone(), &store, &drivers(&FakeDriver::FILE_CAPABILITIES)); - assert!( matches!(resolved.target, Ok(FileTarget::Script)), "{:?}", From 8169278b904b6433f4beaf7a848d7913100be21d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 15:52:40 +0700 Subject: [PATCH 19/24] docs(linux): what happens to a copy that cannot be trimmed --- linux/docs/storage.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/linux/docs/storage.md b/linux/docs/storage.md index 474f04fa7a..a4633dc7b8 100644 --- a/linux/docs/storage.md +++ b/linux/docs/storage.md @@ -257,7 +257,9 @@ can tell whether a re-pinned entry falls in the pages already read. kept hold no history entry or saved query the database no longer has. It attaches the database and deletes the rest in one transaction with `secure_delete` on, optimizing the copy's full-text indexes, so the words - leave its blocks and freed pages together. + leave its blocks and freed pages together. A copy that cannot be trimmed, + being damaged, is deleted, and so is what a crash left of one: an unfinished + copy, a temporary file and the journals of a copy that is gone. - `connections_with_history` lists each connection the history holds under the name of its latest run, so a renamed connection shows once. From d16e45f73d0132b399db5ddfd29b24f9fe70484d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 16:01:18 +0700 Subject: [PATCH 20/24] fix(core-storage): user deletes reach the upgrade copies before the database --- core/crates/storage/src/query_history/mod.rs | 68 +++++++++++++++++--- core/crates/storage/src/saved_queries/mod.rs | 20 +++++- linux/ARCHITECTURE.md | 2 +- linux/docs/storage.md | 11 ++-- 4 files changed, 86 insertions(+), 15 deletions(-) diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index 22604cf27e..a7f5e77093 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -52,7 +52,7 @@ const ENTRY_COLUMNS: &str = "h.id, h.query, h.driver_id, h.connection_id, h.conn h.duration_ms, h.rows_affected, h.success, h.cancelled, h.pinned, h.error"; /// A value for a placeholder of a statement built from parts. -enum Bind { +pub(crate) enum Bind { Text(String), Integer(i64), } @@ -247,6 +247,18 @@ impl QueryHistory { /// Delete the entries and hand back what they held, so the delete /// can be undone with `restore`. pub async fn delete_returning(&self, ids: &[i64]) -> Result, StorageError> { + let from_copies: Vec = ids + .chunks(IDS_PER_STATEMENT) + .map(|chunk| CopyDelete { + table: "history", + sql: format!( + "DELETE FROM main.history WHERE id IN ({})", + vec!["?"; chunk.len()].join(",") + ), + binds: chunk.iter().map(|id| Bind::Integer(*id)).collect(), + }) + .collect(); + trim_upgrade_copies(&self.path, &from_copies).await?; let mut transaction = self.pool.begin().await?; let mut deleted = Vec::with_capacity(ids.len()); for chunk in ids.chunks(IDS_PER_STATEMENT) { @@ -264,7 +276,6 @@ impl QueryHistory { } } transaction.commit().await?; - trim_upgrade_copies(&self.path).await?; deleted.sort_by_key(|entry| entry.id); Ok(deleted) } @@ -302,6 +313,15 @@ impl QueryHistory { } pub async fn clear_all(&self) -> Result { + trim_upgrade_copies( + &self.path, + &[CopyDelete { + table: "history", + sql: "DELETE FROM main.history".to_owned(), + binds: Vec::new(), + }], + ) + .await?; let affected = sqlx::query("DELETE FROM history") .execute(&self.pool) .await? @@ -312,7 +332,6 @@ impl QueryHistory { .execute(&self.pool) .await?; } - trim_upgrade_copies(&self.path).await?; Ok(affected as usize) } @@ -352,7 +371,7 @@ impl QueryHistory { .await?; } // Rows expired here, or unpinned since a copy was made, leave the copies too. - trim_upgrade_copies(&self.path).await?; + trim_upgrade_copies(&self.path, &[]).await?; Ok(report) } @@ -368,6 +387,15 @@ impl QueryHistory { /// Drop every query run on a connection, pinned ones included, for a /// connection the user deleted along with its history. pub async fn delete_for_connection(&self, connection: Uuid) -> Result { + trim_upgrade_copies( + &self.path, + &[CopyDelete { + table: "history", + sql: "DELETE FROM main.history WHERE connection_id = ?".to_owned(), + binds: vec![Bind::Text(connection.to_string())], + }], + ) + .await?; let removed = sqlx::query("DELETE FROM history WHERE connection_id = ?") .bind(connection.to_string()) .execute(&self.pool) @@ -380,7 +408,6 @@ impl QueryHistory { .execute(&self.pool) .await?; } - trim_upgrade_copies(&self.path).await?; Ok(removed) } @@ -438,7 +465,15 @@ impl QueryHistory { // A copy an upgrade kept never holds a history entry or saved query the database no longer has, so // what the user deleted, or the retention window dropped, is gone from it too. One transaction with // secure_delete, so the words leave the full-text blocks and freed pages at once, even after a crash. -pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> { +// A delete the user asked for goes to the copies before the database: when a copy fails, nothing is +// deleted, so the user is told and an Undo never loses what it would put back. +pub(crate) struct CopyDelete { + pub(crate) table: &'static str, + pub(crate) sql: String, + pub(crate) binds: Vec, +} + +pub(crate) async fn trim_upgrade_copies(path: &Path, first: &[CopyDelete]) -> Result<(), StorageError> { crate::upgrade_copies::remove_leftovers_blocking(path)?; let copies = crate::upgrade_copies::copies_blocking(path)?; if copies.is_empty() { @@ -451,14 +486,14 @@ pub(crate) async fn trim_upgrade_copies(path: &Path) -> Result<(), StorageError> for copy in copies { // A copy that cannot be trimmed, being damaged or unreadable, goes rather than keep what the // user deleted. - if trim_copy(©.path, live).await.is_err() { + if trim_copy(©.path, live, first).await.is_err() { crate::upgrade_copies::remove_copy_blocking(©.path)?; } } Ok(()) } -async fn trim_copy(copy: &Path, live: &str) -> Result<(), StorageError> { +async fn trim_copy(copy: &Path, live: &str, first: &[CopyDelete]) -> Result<(), StorageError> { let mut database = SqliteConnectOptions::new() .filename(copy) .journal_mode(sqlx::sqlite::SqliteJournalMode::Delete) @@ -471,6 +506,23 @@ async fn trim_copy(copy: &Path, live: &str) -> Result<(), StorageError> { .execute(&mut database) .await?; let mut transaction = database.begin().await?; + for delete in first { + let present: i64 = + sqlx::query_scalar("SELECT COUNT(*) FROM main.sqlite_master WHERE type = 'table' AND name = ?") + .bind(delete.table) + .fetch_one(&mut *transaction) + .await?; + if present > 0 { + let mut statement = sqlx::query(AssertSqlSafe(delete.sql.clone())); + for bind in &delete.binds { + statement = match bind { + Bind::Text(text) => statement.bind(text.clone()), + Bind::Integer(value) => statement.bind(*value), + }; + } + statement.execute(&mut *transaction).await?; + } + } for table in ["history", "saved_queries"] { let present: i64 = sqlx::query_scalar( "SELECT (SELECT COUNT(*) FROM main.sqlite_master WHERE type = 'table' AND name = ?1) \ diff --git a/core/crates/storage/src/saved_queries/mod.rs b/core/crates/storage/src/saved_queries/mod.rs index d3f9377d64..4a4a00737c 100644 --- a/core/crates/storage/src/saved_queries/mod.rs +++ b/core/crates/storage/src/saved_queries/mod.rs @@ -212,12 +212,20 @@ impl SavedQueries { } pub async fn delete(&self, id: i64) -> Result<(), StorageError> { + crate::query_history::trim_upgrade_copies( + &self.path, + &[crate::query_history::CopyDelete { + table: "saved_queries", + sql: "DELETE FROM main.saved_queries WHERE id = ?".to_owned(), + binds: vec![crate::query_history::Bind::Integer(id)], + }], + ) + .await?; let affected = sqlx::query("DELETE FROM saved_queries WHERE id = ?") .bind(id) .execute(&self.pool) .await? .rows_affected(); - crate::query_history::trim_upgrade_copies(&self.path).await?; match affected { 0 => Err(StorageError::NotFound), _ => Ok(()), @@ -228,12 +236,20 @@ impl SavedQueries { /// connection itself is removed. Its queries name tables only that /// database has. pub async fn delete_for_connection(&self, connection: Uuid) -> Result { + crate::query_history::trim_upgrade_copies( + &self.path, + &[crate::query_history::CopyDelete { + table: "saved_queries", + sql: "DELETE FROM main.saved_queries WHERE connection_id = ?".to_owned(), + binds: vec![crate::query_history::Bind::Text(connection.to_string())], + }], + ) + .await?; let affected = sqlx::query("DELETE FROM saved_queries WHERE connection_id = ?") .bind(connection.to_string()) .execute(&self.pool) .await? .rows_affected(); - crate::query_history::trim_upgrade_copies(&self.path).await?; Ok(affected as usize) } } diff --git a/linux/ARCHITECTURE.md b/linux/ARCHITECTURE.md index 2a9c488e9d..24ed10a09a 100644 --- a/linux/ARCHITECTURE.md +++ b/linux/ARCHITECTURE.md @@ -75,7 +75,7 @@ One second runtime remains. Relm4 runs a component's commands (`sender.command`, A quit freezes the record before it asks the first window, so each window it closes stays on it, and writes it before the process may end. An approved quit then empties the list in memory; a cancelled one keeps only the windows still showing or connecting their session. Shutdown freezes it too: windows that go with the process are not the user closing them. An approved quit can leave the process running while a session closes, and an activate that reaches it then restores the record as a new launch would. -Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`); one GIO reaches without a local path, such as over SFTP, is a script by its name. A tab already bound to the same location comes forward. The location is kept as handed over, as Open SQL File keeps it, and the lookup reads no file system on the GTK thread, so a tab bound through another path, such as a link's target, is not found. Otherwise the script goes to the most recently focused window, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no relational session in front, or while any window is still restoring, connecting or reading its saved tabs, where a tab bound to the file may yet appear, a picker asks: the script waits behind its banner until a relational session joins that window. A session that runs no SQL hands the question to a new picker, and one chosen while it is open in another window takes the scripts there. +Files come from Open With in a file manager or from the command line (`HANDLES_OPEN`). They wait until the restore has read the record, since a file's window adds to it. Each file is resolved off the GTK thread, its path made canonical and matched against the saved connections, then it opens in the window that has it open, in the active window while that shows only the picker, or in a new window. A file whose content type is `application/sql`, or named `.sql`, and whose header carries no engine's signature is a script instead (`FileTarget::Script`); one GIO reaches without a local path, such as over SFTP, is a script by its name. A tab already bound to the same location comes forward. The location is kept as handed over, as Open SQL File keeps it, and the lookup reads no file system on the GTK thread, so a tab bound through another path, such as a link's target, is not found, nor one a window still restoring brings back after the user picked another connection for the script. Otherwise the script goes to the most recently focused window, which reads it as Open SQL File does and opens it in a new editor tab bound to the file in the relational session in front, never running it. With no relational session in front, or while any window is still restoring, connecting or reading its saved tabs, where a tab bound to the file may yet appear, a picker asks: the script waits behind its banner until a relational session joins that window. A session that runs no SQL hands the question to a new picker, and one chosen while it is open in another window takes the scripts there. ## Windows and sessions diff --git a/linux/docs/storage.md b/linux/docs/storage.md index a4633dc7b8..72ee6174ff 100644 --- a/linux/docs/storage.md +++ b/linux/docs/storage.md @@ -251,10 +251,13 @@ can tell whether a re-pinned entry falls in the pages already read. - `delete_for_connection` removes one connection's entries, pinned ones included, for a connection deleted along with its history, then runs `optimize` as a prune does. -- After every deletion (`delete_returning`, `clear_all`, - `delete_for_connection`, `prune` and the saved queries' `delete` and - `delete_for_connection`), `trim_upgrade_copies` makes each copy an upgrade - kept hold no history entry or saved query the database no longer has. It +- A deletion the user asks for (`delete_returning`, `clear_all`, + `delete_for_connection` and the saved queries' `delete` and + `delete_for_connection`) reaches each copy an upgrade kept before the + database, so a copy that fails leaves the database as it was and an Undo + never loses its entries. `prune` trims the copies after its own deletes. + Either way `trim_upgrade_copies` also makes each copy hold no history entry + or saved query the database no longer has. It attaches the database and deletes the rest in one transaction with `secure_delete` on, optimizing the copy's full-text indexes, so the words leave its blocks and freed pages together. A copy that cannot be trimmed, From 4db26625e7bf2b14b509cf2c7e4cc6ae6d918cc2 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 16:10:36 +0700 Subject: [PATCH 21/24] fix(linux): scripts go to the session in front when asked, and a busy copy is kept --- core/crates/storage/src/query_history/mod.rs | 19 ++++++++++-- linux/crates/app/src/ui/session_window/mod.rs | 30 +++++++++++++++---- .../src/ui/session_window/other_databases.rs | 4 +++ .../ui/session_window/session_window_input.rs | 2 ++ 4 files changed, 47 insertions(+), 8 deletions(-) diff --git a/core/crates/storage/src/query_history/mod.rs b/core/crates/storage/src/query_history/mod.rs index a7f5e77093..b4454480ee 100644 --- a/core/crates/storage/src/query_history/mod.rs +++ b/core/crates/storage/src/query_history/mod.rs @@ -486,13 +486,28 @@ pub(crate) async fn trim_upgrade_copies(path: &Path, first: &[CopyDelete]) -> Re for copy in copies { // A copy that cannot be trimmed, being damaged or unreadable, goes rather than keep what the // user deleted. - if trim_copy(©.path, live, first).await.is_err() { - crate::upgrade_copies::remove_copy_blocking(©.path)?; + match trim_copy(©.path, live, first).await { + Ok(()) => {} + // Another trim holds it: the copy is fine, and this one tries again next time. + Err(error) if is_busy(&error) => return Err(error), + Err(_) => crate::upgrade_copies::remove_copy_blocking(©.path)?, } } Ok(()) } +// SQLITE_BUSY and SQLITE_LOCKED, with their extended codes. +fn is_busy(error: &StorageError) -> bool { + let StorageError::Database(error) = error else { + return false; + }; + error + .as_database_error() + .and_then(|database| database.code()) + .and_then(|code| code.parse::().ok()) + .is_some_and(|code| matches!(code & 0xff, 5 | 6)) +} + async fn trim_copy(copy: &Path, live: &str, first: &[CopyDelete]) -> Result<(), StorageError> { let mut database = SqliteConnectOptions::new() .filename(copy) diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index 79d92812aa..869d3794dc 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -442,7 +442,7 @@ impl SessionWindow { SessionWindowInput::OpenFile(file) => self.on_open_file(file), SessionWindowInput::FileResolved(resolved) => self.on_file_resolved(*resolved), SessionWindowInput::OpenScript(file) => self.on_open_script(file), - SessionWindowInput::ScriptLoaded { file, text } => self.on_script_loaded(file, text), + SessionWindowInput::ScriptLoaded { file, text, target } => self.on_script_loaded(file, text, target), SessionWindowInput::ScriptsFor { key, scripts } => self.on_scripts_for(key, scripts), SessionWindowInput::DropWaitingScripts => { self.imp().waiting_scripts.borrow_mut().clear(); @@ -1127,6 +1127,10 @@ impl SessionWindow { let imp = self.imp(); !imp.restoring.try_borrow().is_ok_and(|restoring| restoring.is_empty()) || !imp.connects.try_borrow().is_ok_and(|connects| connects.is_empty()) + || !imp + .sessions + .try_borrow() + .is_ok_and(|sessions| sessions.iter().all(|open| !open.databases.is_opening())) || self.workspaces().iter().any(Workspace::tabs_loading) } @@ -1158,18 +1162,23 @@ impl SessionWindow { } // Read the way Open SQL File reads, with the same limit and messages. + // The session in front now is the one it goes to, even if another comes forward during a slow read. fn on_open_script(&self, file: gio::File) { let Some(input) = self.input() else { return; }; + let target = self + .workspace() + .filter(|workspace| workspace.data_model() == Some(DataModel::Relational)) + .and_then(|workspace| workspace.key()); glib::spawn_future_local(async move { let text = crate::ui::editor::load_text(&file).await; - input.deliver(SessionWindowInput::ScriptLoaded { file, text }); + input.deliver(SessionWindowInput::ScriptLoaded { file, text, target }); }); } // A script opens in the relational session in front and is never run. - fn on_script_loaded(&self, file: gio::File, text: Result) { + fn on_script_loaded(&self, file: gio::File, text: Result, target: Option) { let text = match text { Ok(text) => text, Err(reason) => { @@ -1184,8 +1193,13 @@ impl SessionWindow { if self.focus_bound_tab(&file) { return; } - let front = self - .workspace() + let chosen = target.and_then(|key| { + self.workspaces() + .into_iter() + .find(|workspace| workspace.key().as_ref() == Some(&key)) + }); + let front = chosen + .or_else(|| self.workspace()) .filter(|workspace| workspace.data_model() == Some(DataModel::Relational)) .and_then(|workspace| workspace.sender()); match front { @@ -1220,7 +1234,11 @@ impl SessionWindow { return; }; for (file, text) in scripts { - window.send(SessionWindowInput::ScriptLoaded { file, text: Ok(text) }); + window.send(SessionWindowInput::ScriptLoaded { + file, + text: Ok(text), + target: None, + }); } } diff --git a/linux/crates/app/src/ui/session_window/other_databases.rs b/linux/crates/app/src/ui/session_window/other_databases.rs index a55fb12d59..874780e867 100644 --- a/linux/crates/app/src/ui/session_window/other_databases.rs +++ b/linux/crates/app/src/ui/session_window/other_databases.rs @@ -44,6 +44,10 @@ struct Opening { } impl OtherDatabases { + pub(super) fn is_opening(&self) -> bool { + !self.opening.is_empty() + } + /// The databases open or opening, by name. pub(super) fn names(&self) -> Vec { let open = self.open.iter().filter_map(|open| match open.claim.key() { diff --git a/linux/crates/app/src/ui/session_window/session_window_input.rs b/linux/crates/app/src/ui/session_window/session_window_input.rs index 61a786fc67..fce35aac6f 100644 --- a/linux/crates/app/src/ui/session_window/session_window_input.rs +++ b/linux/crates/app/src/ui/session_window/session_window_input.rs @@ -31,6 +31,8 @@ pub(crate) enum SessionWindowInput { ScriptLoaded { file: gio::File, text: Result, + /// The session in front when the read began, which takes the script if it is still open. + target: Option, }, /// The waiting scripts banner's Cancel. DropWaitingScripts, From 92b253f7383303567ee98bc2b563d8bd35ce11f8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 20:52:17 +0700 Subject: [PATCH 22/24] test(linux): read a toast's title as the optional it is --- linux/crates/app/src/ui/session_window/tests/open_file.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/crates/app/src/ui/session_window/tests/open_file.rs b/linux/crates/app/src/ui/session_window/tests/open_file.rs index 6839d1c9de..1ac3800ae7 100644 --- a/linux/crates/app/src/ui/session_window/tests/open_file.rs +++ b/linux/crates/app/src/ui/session_window/tests/open_file.rs @@ -283,7 +283,7 @@ fn an_unreadable_sql_file_says_why_and_opens_nothing() { .toasts .borrow() .iter() - .any(|toast| toast.title().contains("not UTF-8 text")) + .any(|toast| toast.title().is_some_and(|title| title.contains("not UTF-8 text"))) }) .expect("a toast saying why"); drain_main_context(); From 1e7ed0781eb4565648ef2611f9ec08c7c87f754c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Thu, 5 Nov 2026 21:00:39 +0700 Subject: [PATCH 23/24] refactor(linux): script inputs in one enum, so the window's input handler stays short --- .../crates/app/src/application/open_files.rs | 4 +-- linux/crates/app/src/ui/mod.rs | 2 +- linux/crates/app/src/ui/session_window/mod.rs | 34 ++++++++++-------- .../ui/session_window/session_window_input.rs | 36 +++++++++++-------- .../src/ui/session_window/tests/open_file.rs | 2 +- 5 files changed, 45 insertions(+), 33 deletions(-) diff --git a/linux/crates/app/src/application/open_files.rs b/linux/crates/app/src/application/open_files.rs index 1126780360..850b3faa4f 100644 --- a/linux/crates/app/src/application/open_files.rs +++ b/linux/crates/app/src/application/open_files.rs @@ -4,7 +4,7 @@ use tablepro_session::SessionKey; use super::{TableProApplication, WindowLaunch}; use crate::services::file_resolution::{FileTarget, ResolvedFile, resolve_file_blocking}; -use crate::ui::{SessionWindow, SessionWindowInput}; +use crate::ui::{ScriptInput, SessionWindow, SessionWindowInput}; impl TableProApplication { /// Open each file the file manager or the command line hands over. @@ -99,7 +99,7 @@ impl TableProApplication { }; match window { Some(window) => { - window.send(SessionWindowInput::OpenScript(file)); + window.send(SessionWindowInput::Script(ScriptInput::Open(file))); window.present(); } None => tracing::warn!("no window to open a SQL file in"), diff --git a/linux/crates/app/src/ui/mod.rs b/linux/crates/app/src/ui/mod.rs index 8140ea2bb8..d442bcf312 100644 --- a/linux/crates/app/src/ui/mod.rs +++ b/linux/crates/app/src/ui/mod.rs @@ -62,6 +62,6 @@ pub(crate) use menu_label::menu_label; pub(crate) use plain_toast::plain_toast; pub(crate) use preferences_dialog::PreferencesDialog; pub(crate) use session_window::{ - LaunchNotice, LibraryEdit, MovedTab, SessionWindow, SessionWindowInit, SessionWindowInput, + LaunchNotice, LibraryEdit, MovedTab, ScriptInput, SessionWindow, SessionWindowInit, SessionWindowInput, }; pub(crate) use structure_tab::review_dialog; diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index 869d3794dc..b7796cf5fe 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -68,7 +68,7 @@ pub(super) use self::picker_page::PickerPage; use self::resolve_file_identity::{FileResolveError, FileTarget, ResolvedFile, resolve_file_blocking}; use self::session_page::{FailedConnect, OpenSession, PendingConnect, SessionPage, TakenFailure, source_key}; use self::session_window_command::{AttemptError, ConnectFailure, Connected, SessionWindowCommand}; -pub(crate) use self::session_window_input::{LibraryEdit, MovedTab, SessionWindowInput}; +pub(crate) use self::session_window_input::{LibraryEdit, MovedTab, ScriptInput, SessionWindowInput}; use crate::application::{AppServices, WindowHost, WindowLaunch}; use crate::runtime::{DeliverInput, InputSender}; use crate::services::session_context::SessionContext; @@ -172,7 +172,7 @@ impl SessionWindow { let drop_input = input.clone(); picker .script_banner() - .connect_button_clicked(move |_| drop_input.deliver(SessionWindowInput::DropWaitingScripts)); + .connect_button_clicked(move |_| drop_input.deliver(SessionWindowInput::Script(ScriptInput::DropWaiting))); let window = self.downgrade(); imp.list_observer.set(Some(list.connect_state_changed(move |_| { if let Some(window) = window.upgrade() { @@ -441,13 +441,7 @@ impl SessionWindow { SessionWindowInput::OpenSaved(saved) => self.on_open_saved(*saved), SessionWindowInput::OpenFile(file) => self.on_open_file(file), SessionWindowInput::FileResolved(resolved) => self.on_file_resolved(*resolved), - SessionWindowInput::OpenScript(file) => self.on_open_script(file), - SessionWindowInput::ScriptLoaded { file, text, target } => self.on_script_loaded(file, text, target), - SessionWindowInput::ScriptsFor { key, scripts } => self.on_scripts_for(key, scripts), - SessionWindowInput::DropWaitingScripts => { - self.imp().waiting_scripts.borrow_mut().clear(); - self.show_waiting_scripts(); - } + SessionWindowInput::Script(input) => self.on_script_input(input), SessionWindowInput::Restore(key) => self.on_restore(key), SessionWindowInput::RestoreSaved { id, saved } => self.on_restore_saved(id, saved.map(|saved| *saved)), SessionWindowInput::RestoreUnread(key) => self.end_restore(&key), @@ -1162,6 +1156,18 @@ impl SessionWindow { } // Read the way Open SQL File reads, with the same limit and messages. + fn on_script_input(&self, input: ScriptInput) { + match input { + ScriptInput::Open(file) => self.on_open_script(file), + ScriptInput::Loaded { file, text, target } => self.on_script_loaded(file, text, target), + ScriptInput::For { key, scripts } => self.on_scripts_for(key, scripts), + ScriptInput::DropWaiting => { + self.imp().waiting_scripts.borrow_mut().clear(); + self.show_waiting_scripts(); + } + } + } + // The session in front now is the one it goes to, even if another comes forward during a slow read. fn on_open_script(&self, file: gio::File) { let Some(input) = self.input() else { @@ -1173,7 +1179,7 @@ impl SessionWindow { .and_then(|workspace| workspace.key()); glib::spawn_future_local(async move { let text = crate::ui::editor::load_text(&file).await; - input.deliver(SessionWindowInput::ScriptLoaded { file, text, target }); + input.deliver(SessionWindowInput::Script(ScriptInput::Loaded { file, text, target })); }); } @@ -1234,11 +1240,11 @@ impl SessionWindow { return; }; for (file, text) in scripts { - window.send(SessionWindowInput::ScriptLoaded { + window.send(SessionWindowInput::Script(ScriptInput::Loaded { file, text: Ok(text), target: None, - }); + })); } } @@ -1266,10 +1272,10 @@ impl SessionWindow { return; } let scripts = self.imp().waiting_scripts.take(); - owner.send(SessionWindowInput::ScriptsFor { + owner.send(SessionWindowInput::Script(ScriptInput::For { key: key.clone(), scripts, - }); + })); self.show_waiting_scripts(); } diff --git a/linux/crates/app/src/ui/session_window/session_window_input.rs b/linux/crates/app/src/ui/session_window/session_window_input.rs index fce35aac6f..eb161b86dc 100644 --- a/linux/crates/app/src/ui/session_window/session_window_input.rs +++ b/linux/crates/app/src/ui/session_window/session_window_input.rs @@ -26,21 +26,8 @@ pub(crate) enum SessionWindowInput { OpenFile(gio::File), /// The file resolved, off the GTK thread, to the session it opens. FileResolved(Box), - /// Open a SQL script in an editor tab, from Files or the command line. - OpenScript(gio::File), - ScriptLoaded { - file: gio::File, - text: Result, - /// The session in front when the read began, which takes the script if it is still open. - target: Option, - }, - /// The waiting scripts banner's Cancel. - DropWaitingScripts, - /// Scripts another window's user chose this window's session for. - ScriptsFor { - key: SessionKey, - scripts: Vec<(gio::File, String)>, - }, + /// A SQL script from Files or the command line, on its way to an editor tab. + Script(ScriptInput), /// Open one of the sessions the window was opened to restore. Restore(SessionKey), /// The saved connections could not be read for this restored key, @@ -223,3 +210,22 @@ pub(crate) enum LibraryEdit { Change(LibraryMove), SetFavourite(Uuid, bool), } + +#[derive(Debug)] +pub(crate) enum ScriptInput { + /// Open it in an editor tab. + Open(gio::File), + Loaded { + file: gio::File, + text: Result, + /// The session in front when the read began, which takes the script if it is still open. + target: Option, + }, + /// The waiting scripts banner's Cancel. + DropWaiting, + /// Scripts another window's user chose this window's session for. + For { + key: SessionKey, + scripts: Vec<(gio::File, String)>, + }, +} diff --git a/linux/crates/app/src/ui/session_window/tests/open_file.rs b/linux/crates/app/src/ui/session_window/tests/open_file.rs index 1ac3800ae7..a466a3a718 100644 --- a/linux/crates/app/src/ui/session_window/tests/open_file.rs +++ b/linux/crates/app/src/ui/session_window/tests/open_file.rs @@ -312,7 +312,7 @@ fn a_sql_file_already_open_brings_its_tab_forward() { wait_until(WAIT, || !in_front()).expect("another tab in front"); test.app.open(std::slice::from_ref(&file), ""); - wait_until(WAIT, || in_front()).expect("the script's tab in front"); + wait_until(WAIT, in_front).expect("the script's tab in front"); let bound = editors(&test.window) .iter() From a4a22be57a82b34e7c1a501774626cb34b022c7b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 6 Nov 2026 03:30:31 +0700 Subject: [PATCH 24/24] fix(linux): read the import preview's list-item rows in its test, and leave media types to the desktop entry --- .../app/src/ui/session_window/connections.rs | 21 ++++++++++--- linux/crates/app/tests/desktop_entry.rs | 31 ------------------- .../app.tablepro.TablePro.metainfo.xml.in.in | 2 -- 3 files changed, 16 insertions(+), 38 deletions(-) diff --git a/linux/crates/app/src/ui/session_window/connections.rs b/linux/crates/app/src/ui/session_window/connections.rs index a75493bc83..fec6f91493 100644 --- a/linux/crates/app/src/ui/session_window/connections.rs +++ b/linux/crates/app/src/ui/session_window/connections.rs @@ -1273,19 +1273,30 @@ mod tests { }); let alert = wait_for_alert(&test.window); let list = alert.extra_child().expect("the dialog's rows"); - let mut rows: Vec = Vec::new(); + // A list view's row is its item widget, which holds the title and subtitle labels. + let mut rows: Vec<(String, String)> = Vec::new(); wait_until(WAIT, || { rows = crate::test_support::descendants(&list) .into_iter() - .filter_map(|widget| widget.downcast::().ok()) - .filter(|row| !row.title().is_empty()) + .filter(|widget| widget.css_name() == "row") + .filter_map(|row| { + let labels: Vec = crate::test_support::descendants(&row) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .map(|label| label.label().to_string()) + .collect(); + match labels.as_slice() { + [title, subtitle] if !title.is_empty() => Some((title.clone(), subtitle.clone())), + _ => None, + } + }) .collect(); !rows.is_empty() }) .expect("the rows"); assert_eq!(rows.len(), 1); - assert_eq!(rows[0].title(), format!("{} (imported)", test.saved.name)); - let subtitle = rows[0].subtitle().unwrap_or_default(); + assert_eq!(rows[0].0, format!("{} (imported)", test.saved.name)); + let subtitle = &rows[0].1; assert!(subtitle.contains("app@db.elsewhere.example"), "{subtitle}"); assert!(subtitle.contains("Connects without TLS"), "{subtitle}"); assert!(!listed(&test, lookalike.id), "the preview wrote the list"); diff --git a/linux/crates/app/tests/desktop_entry.rs b/linux/crates/app/tests/desktop_entry.rs index 890fe0f78e..b115540120 100644 --- a/linux/crates/app/tests/desktop_entry.rs +++ b/linux/crates/app/tests/desktop_entry.rs @@ -13,12 +13,6 @@ fn entry() -> Result { Ok(entry) } -fn metainfo() -> std::io::Result { - std::fs::read_to_string( - Path::new(env!("CARGO_MANIFEST_DIR")).join("../../data/app.tablepro.TablePro.metainfo.xml.in.in"), - ) -} - fn list(entry: &glib::KeyFile, group: &str, key: &str) -> Result, glib::Error> { Ok(entry .string_list(group, key)? @@ -51,31 +45,6 @@ fn sql_scripts_are_offered_to_tablepro() { ); } -// Software centres read the media types from the metainfo, not the entry. -#[test] -fn the_metainfo_provides_every_type_the_entry_takes() { - let entry = entry().expect("the desktop entry parses"); - let xml = metainfo().expect("the metainfo reads"); - - let mut taken = list(&entry, MAIN, "MimeType").expect("a MimeType list"); - let provides = xml - .split_once("") - .and_then(|(_, rest)| rest.split_once("")) - .map(|(provides, _)| provides) - .expect("a provides element"); - let mut provided: Vec = provides - .split("") - .skip(1) - .filter_map(|rest| rest.split_once("")) - .map(|(media_type, _)| media_type.trim().to_owned()) - .collect(); - taken.retain(|media_type| !media_type.is_empty()); - taken.sort(); - provided.sort(); - - assert_eq!(provided, taken); -} - #[test] fn files_reach_the_running_application() { let entry = entry().expect("the desktop entry parses"); diff --git a/linux/data/app.tablepro.TablePro.metainfo.xml.in.in b/linux/data/app.tablepro.TablePro.metainfo.xml.in.in index 157e21fed4..fbe5359ec0 100644 --- a/linux/data/app.tablepro.TablePro.metainfo.xml.in.in +++ b/linux/data/app.tablepro.TablePro.metainfo.xml.in.in @@ -65,8 +65,6 @@ tablepro - application/vnd.sqlite3 - application/sql