diff --git a/src-tauri/src/ai_activity.rs b/src-tauri/src/ai_activity.rs index f2b583e5..8607c737 100644 --- a/src-tauri/src/ai_activity.rs +++ b/src-tauri/src/ai_activity.rs @@ -482,7 +482,7 @@ fn has_complete_end_before_terminal_statement(upper_sql: &str) -> bool { /// string-literal, comment, and quoted-identifier bytes with whitespace, so /// any `;` that survives here is a real statement terminator under the /// escaping interpretation used to produce `stripped`. -fn has_trailing_statements(stripped: &str) -> bool { +pub(crate) fn has_trailing_statements(stripped: &str) -> bool { let mut found_semi = false; for c in stripped.chars() { if found_semi && !c.is_whitespace() { diff --git a/src-tauri/src/commands.rs b/src-tauri/src/commands.rs index f052c160..3dc73382 100644 --- a/src-tauri/src/commands.rs +++ b/src-tauri/src/commands.rs @@ -3548,6 +3548,40 @@ pub async fn cancel_query( cancel_query_impl(&state, &connection_id) } +/// Payload for the `database-dropped` event, emitted after a `DROP DATABASE` +/// statement succeeds so a listener can reconcile the connection's database +/// selection instead of leaving the dropped database in the sidebar until the +/// next reconnect (#525). +// `connectionId` rather than `connection_id`: the `connection-health-failed` +// event already carries this same field in camelCase, and matching the field +// name for the same concept matters more than matching the neighbouring +// `batch-statement-complete` payload, which happens to use snake_case. +#[derive(serde::Serialize, Clone)] +#[serde(rename_all = "camelCase")] +struct DatabaseDroppedEvent<'a> { + connection_id: &'a str, + database: &'a str, +} + +/// Announces that `database` no longer exists on the server behind +/// `connection_id`. Emitting is best-effort: a listener that missed the event +/// only means the sidebar stays stale until the next manual refresh, which is +/// the pre-#525 behaviour, so a failed emit must not fail the query. +fn emit_database_dropped(app: &AppHandle, connection_id: &str, database: &str) { + log::info!( + "DROP DATABASE detected on connection {}: '{}'", + connection_id, + database + ); + let _ = app.emit( + "database-dropped", + DatabaseDroppedEvent { + connection_id, + database, + }, + ); +} + #[tauri::command] pub async fn execute_query( app: AppHandle, @@ -3571,6 +3605,10 @@ pub async fn execute_query( let expanded_params = expand_k8s_connection_params(&app, &expanded_params).await?; let params = resolve_connection_params_with_id(&expanded_params, &connection_id)?; + // Detected before the spawn, which takes ownership of `sanitized_query`. + // Cheap: only allocates when the statement really is a DROP DATABASE. + let dropped = crate::sql_database_statements::dropped_database(&sanitized_query); + let drv = driver_for(&saved_conn.params.driver).await?; let task = tokio::spawn(async move { drv.execute_query( @@ -3596,6 +3634,9 @@ pub async fn execute_query( "Query executed successfully, returned {} rows", query_result.rows.len() ); + if let Some(database) = &dropped { + emit_database_dropped(&app, &connection_id, database); + } Ok(query_result) } Ok(Err(e)) => { @@ -3653,6 +3694,14 @@ pub async fn execute_query_batch( let sanitized_queries: Vec = queries.iter().map(|q| sanitize_user_query(q)).collect(); + // One entry per statement, computed before the spawn takes ownership of + // `sanitized_queries`. Index-aligned with the results returned below, the + // same assumption the `batch-statement-complete` event already relies on. + let dropped_per_statement: Vec> = sanitized_queries + .iter() + .map(|q| crate::sql_database_statements::dropped_database(q)) + .collect(); + let saved_conn = find_connection_by_id(&app, &connection_id)?; let expanded_params = expand_ssh_connection_params(&app, &saved_conn.params).await?; let expanded_params = expand_k8s_connection_params(&app, &expanded_params).await?; @@ -3707,6 +3756,16 @@ pub async fn execute_query_batch( batch_results.len() - success_count, batch_results.len() ); + // A batch reports per-statement outcomes, so only announce drops + // whose own statement succeeded; a failed DROP leaves the database + // in place. + for (dropped, statement) in dropped_per_statement.iter().zip(&batch_results) { + if let Some(database) = dropped { + if statement.result.is_some() { + emit_database_dropped(&app, &connection_id, database); + } + } + } Ok(batch_results) } Ok(Err(e)) => { diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 0be98311..b4432d57 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -69,6 +69,7 @@ pub mod preferences; pub mod query_history; #[cfg(test)] pub mod query_history_tests; +pub mod sql_database_statements; pub mod saved_queries; #[cfg(test)] pub mod saved_queries_tests; diff --git a/src-tauri/src/sql_database_statements.rs b/src-tauri/src/sql_database_statements.rs new file mode 100644 index 00000000..6a68367e --- /dev/null +++ b/src-tauri/src/sql_database_statements.rs @@ -0,0 +1,116 @@ +//! Recognises the few SQL statements that change which databases exist, so the +//! app can keep the sidebar in sync with the server instead of waiting for a +//! manual refresh (#525, following #518 and #524). +//! +//! Everything here is deliberately narrow and fails closed: a statement that +//! isn't unambiguously recognised returns `None` rather than a guess. A missed +//! detection just leaves the sidebar stale until the next refresh, which is +//! today's behaviour anyway; a wrong detection would tell the user a database +//! is gone when it isn't, which is worse than doing nothing. + +use crate::ai_activity::{has_trailing_statements, strip_strings_and_comments}; + +#[cfg(test)] +mod tests; + +/// The database targeted by a `DROP DATABASE` / `DROP SCHEMA` statement. +/// +/// Returns `None` unless the input is exactly one such statement. In +/// particular, a multi-statement payload returns `None`: which statement +/// actually succeeded is the caller's problem to disambiguate, not this +/// function's to guess. +/// +/// The statement must also start with `DROP`, so a leading comment (`-- note` +/// before the statement) is not recognised. Callers pass the statement they +/// just executed, which in practice starts at the keyword; and a miss here only +/// costs a stale sidebar until the next refresh. +pub fn dropped_database(sql: &str) -> Option { + // Only used to reject multi-statement payloads and to ignore text inside + // comments and string literals. The walk below reads the original string, + // because stripping also blanks out quoted identifiers -- which is exactly + // where the database name lives. + if has_trailing_statements(&strip_strings_and_comments(sql)) { + return None; + } + + let rest = after_keyword(sql, "DROP")?; + // MySQL and Postgres both accept SCHEMA as a synonym of DATABASE here. + let rest = after_keyword(rest, "DATABASE").or_else(|| after_keyword(rest, "SCHEMA"))?; + let rest = match after_keyword(rest, "IF") { + Some(after_if) => after_keyword(after_if, "EXISTS")?, + None => rest, + }; + + identifier(rest) +} + +/// The text following an ASCII SQL keyword at the start of `s` (after leading +/// whitespace), matched case-insensitively and left-trimmed. +/// +/// Requires a word boundary after the keyword -- whitespace, an identifier +/// quote, or end of input -- so `DATABASEX` never matches `DATABASE`. +fn after_keyword<'a>(s: &'a str, keyword: &str) -> Option<&'a str> { + let s = s.trim_start(); + // Byte slicing is safe here: the keyword is pure ASCII, so if the leading + // bytes match it they are all single-byte characters and `keyword.len()` + // lands on a character boundary. + if s.len() < keyword.len() + || !s.as_bytes()[..keyword.len()].eq_ignore_ascii_case(keyword.as_bytes()) + { + return None; + } + let tail = &s[keyword.len()..]; + match tail.chars().next() { + None => Some(tail), + Some(c) if c.is_whitespace() || is_quote(c) => Some(tail.trim_start()), + Some(_) => None, + } +} + +/// Whether `c` opens a quoted identifier in any dialect the app supports. +fn is_quote(c: char) -> bool { + matches!(c, '`' | '"' | '[') +} + +/// The single identifier `s` consists of, unquoted. +/// +/// Accepts backticks (MySQL/MariaDB), double quotes (Postgres, SQL standard) +/// and brackets (SQL Server), or a bare name. Anything other than whitespace +/// and an optional `;` after the identifier returns `None`, so +/// `DROP DATABASE foo bar` yields nothing rather than silently taking `foo`. +/// +/// A doubled quote inside a quoted identifier (a literal backtick in the name) +/// is not handled: it ends the name early, the leftover fails the trailing +/// check, and the result is `None`. That shape is vanishingly rare for a +/// database name, and failing closed costs a missed detection rather than a +/// truncated name. +fn identifier(s: &str) -> Option { + let name = match s.chars().next()? { + quote @ ('`' | '"') => delimited(s, quote, quote)?, + '[' => delimited(s, '[', ']')?, + _ => bare(s)?, + }; + Some(name.to_string()) +} + +/// The text between `open` and the next `close`, if what follows it is only +/// whitespace and an optional statement terminator. +fn delimited(s: &str, open: char, close: char) -> Option<&str> { + let body = s.strip_prefix(open)?; + let (name, after) = body.split_once(close)?; + only_terminator(after).then_some(name) +} + +/// A leading unquoted identifier: alphanumerics, `_` and `$` (accepted by +/// MySQL, and present in some legacy schema names). +fn bare(s: &str) -> Option<&str> { + let end = s + .find(|c: char| !(c.is_alphanumeric() || c == '_' || c == '$')) + .unwrap_or(s.len()); + let (name, after) = s.split_at(end); + (!name.is_empty() && only_terminator(after)).then_some(name) +} + +fn only_terminator(s: &str) -> bool { + matches!(s.trim(), "" | ";") +} diff --git a/src-tauri/src/sql_database_statements/tests.rs b/src-tauri/src/sql_database_statements/tests.rs new file mode 100644 index 00000000..1b3b569b --- /dev/null +++ b/src-tauri/src/sql_database_statements/tests.rs @@ -0,0 +1,217 @@ +use super::*; + +#[test] +fn recognises_a_plain_drop() { + assert_eq!(dropped_database("DROP DATABASE foo"), Some("foo".into())); +} + +#[test] +fn keyword_matching_is_case_insensitive() { + assert_eq!(dropped_database("drop database foo"), Some("foo".into())); + assert_eq!(dropped_database("Drop Database foo"), Some("foo".into())); +} + +#[test] +fn accepts_schema_as_a_synonym() { + assert_eq!(dropped_database("DROP SCHEMA foo"), Some("foo".into())); +} + +#[test] +fn accepts_if_exists() { + assert_eq!( + dropped_database("DROP DATABASE IF EXISTS foo"), + Some("foo".into()) + ); +} + +#[test] +fn if_exists_without_a_name_is_not_a_drop() { + assert_eq!(dropped_database("DROP DATABASE IF EXISTS"), None); + assert_eq!(dropped_database("DROP DATABASE IF"), None); +} + +#[test] +fn a_name_starting_with_if_is_still_a_name() { + assert_eq!( + dropped_database("DROP DATABASE if_logs"), + Some("if_logs".into()) + ); + assert_eq!( + dropped_database("DROP DATABASE ifexists"), + Some("ifexists".into()) + ); +} + +#[test] +fn tolerates_surrounding_whitespace_and_terminator() { + assert_eq!(dropped_database("DROP DATABASE foo;"), Some("foo".into())); + assert_eq!( + dropped_database(" DROP DATABASE foo ; "), + Some("foo".into()) + ); +} + +#[test] +fn unquotes_every_dialect_quote() { + assert_eq!( + dropped_database("DROP DATABASE `my db`"), + Some("my db".into()) + ); + assert_eq!( + dropped_database("DROP DATABASE \"my db\""), + Some("my db".into()) + ); + assert_eq!( + dropped_database("DROP DATABASE [my db]"), + Some("my db".into()) + ); +} + +#[test] +fn a_leading_comment_is_not_recognised() { + // Documented limitation: the statement must start with DROP. Failing + // closed here only costs a stale sidebar until the next refresh. + assert_eq!(dropped_database("-- cleanup\nDROP DATABASE foo"), None); + assert_eq!(dropped_database("/* cleanup */ DROP DATABASE foo"), None); +} + +#[test] +fn keeps_a_non_ascii_name_intact() { + assert_eq!( + dropped_database("DROP DATABASE `groß`"), + Some("groß".into()) + ); +} + +#[test] +fn other_statements_are_not_drops() { + assert_eq!(dropped_database("DROP TABLE foo"), None); + assert_eq!(dropped_database("CREATE DATABASE foo"), None); + assert_eq!(dropped_database("SELECT 1"), None); + assert_eq!(dropped_database(""), None); +} + +#[test] +fn a_database_name_inside_a_string_is_not_a_drop() { + assert_eq!(dropped_database("SELECT 'DROP DATABASE foo'"), None); +} + +#[test] +fn requires_a_word_boundary_after_the_keyword() { + assert_eq!(dropped_database("DROP DATABASEX foo"), None); + assert_eq!(dropped_database("DROPX DATABASE foo"), None); +} + +#[test] +fn refuses_to_guess_in_a_multi_statement_payload() { + assert_eq!(dropped_database("DROP DATABASE foo; SELECT 1"), None); + assert_eq!(dropped_database("SELECT 1; DROP DATABASE foo"), None); +} + +#[test] +fn refuses_anything_trailing_the_identifier() { + assert_eq!(dropped_database("DROP DATABASE foo bar"), None); + assert_eq!(dropped_database("DROP DATABASE foo;;"), None); +} + +#[test] +fn an_unclosed_quote_is_not_a_drop() { + assert_eq!(dropped_database("DROP DATABASE `foo"), None); + assert_eq!(dropped_database("DROP DATABASE [foo"), None); +} + +// --- Helpers, tested directly (rust.md rule 5) ------------------------------- + +#[test] +fn after_keyword_matches_case_insensitively_and_trims() { + assert_eq!(after_keyword("DROP foo", "DROP"), Some("foo")); + assert_eq!(after_keyword("drop foo", "DROP"), Some("foo")); + assert_eq!(after_keyword(" DROP foo", "DROP"), Some("foo")); +} + +#[test] +fn after_keyword_accepts_a_quote_as_word_boundary() { + // No space between keyword and a quoted identifier is still valid SQL. + assert_eq!(after_keyword("DATABASE`db`", "DATABASE"), Some("`db`")); + assert_eq!(after_keyword("DATABASE\"db\"", "DATABASE"), Some("\"db\"")); + assert_eq!(after_keyword("DATABASE[db]", "DATABASE"), Some("[db]")); +} + +#[test] +fn after_keyword_returns_empty_when_the_keyword_ends_the_input() { + assert_eq!(after_keyword("DROP", "DROP"), Some("")); +} + +#[test] +fn after_keyword_requires_a_word_boundary() { + assert_eq!(after_keyword("DROPX foo", "DROP"), None); + assert_eq!(after_keyword("DROP_TABLE foo", "DROP"), None); +} + +#[test] +fn after_keyword_rejects_a_shorter_input_and_a_mismatch() { + assert_eq!(after_keyword("DRO", "DROP"), None); + assert_eq!(after_keyword("", "DROP"), None); + assert_eq!(after_keyword("SELECT foo", "DROP"), None); +} + +#[test] +fn is_quote_covers_the_three_identifier_quotes_only() { + assert!(is_quote('`')); + assert!(is_quote('"')); + assert!(is_quote('[')); + // A single quote opens a string literal, not an identifier. + assert!(!is_quote('\'')); + assert!(!is_quote('a')); +} + +#[test] +fn only_terminator_allows_blank_and_a_single_semicolon() { + assert!(only_terminator("")); + assert!(only_terminator(" ")); + assert!(only_terminator(";")); + assert!(only_terminator(" ; ")); + assert!(!only_terminator(";;")); + assert!(!only_terminator("bar")); +} + +#[test] +fn bare_reads_an_unquoted_identifier() { + assert_eq!(bare("foo"), Some("foo")); + assert_eq!(bare("foo;"), Some("foo")); + assert_eq!(bare("my_db"), Some("my_db")); + // `$` is accepted by MySQL and appears in some legacy schema names. + assert_eq!(bare("foo$bar"), Some("foo$bar")); +} + +#[test] +fn bare_rejects_trailing_content_and_empty_names() { + assert_eq!(bare("foo bar"), None); + assert_eq!(bare(""), None); + assert_eq!(bare(";"), None); + assert_eq!(bare("`foo`"), None); +} + +#[test] +fn delimited_extracts_between_matching_quotes() { + assert_eq!(delimited("`foo`", '`', '`'), Some("foo")); + assert_eq!(delimited("`my db`", '`', '`'), Some("my db")); + assert_eq!(delimited("[my db]", '[', ']'), Some("my db")); + assert_eq!(delimited("`foo`;", '`', '`'), Some("foo")); +} + +#[test] +fn delimited_fails_closed_on_unclosed_quotes_and_trailing_content() { + assert_eq!(delimited("`foo", '`', '`'), None); + assert_eq!(delimited("`foo` bar", '`', '`'), None); + assert_eq!(delimited("foo", '`', '`'), None); +} + +#[test] +fn identifier_dispatches_on_the_opening_character() { + assert_eq!(identifier("foo"), Some("foo".into())); + assert_eq!(identifier("`a b`"), Some("a b".into())); + assert_eq!(identifier("\"a b\""), Some("a b".into())); + assert_eq!(identifier("[a b]"), Some("a b".into())); + assert_eq!(identifier(""), None); +} diff --git a/src/contexts/DatabaseContext.ts b/src/contexts/DatabaseContext.ts index c8eaa3f8..5078d9a0 100644 --- a/src/contexts/DatabaseContext.ts +++ b/src/contexts/DatabaseContext.ts @@ -159,7 +159,10 @@ export interface DatabaseContextType { loadDatabaseData: (database: string, connectionId?: string) => Promise; refreshDatabaseData: (database: string, connectionId?: string) => Promise; setSelectedDatabases: (databases: string[], connectionId?: string) => void; - refreshDatabaseSelection: (connectionId: string) => Promise; + refreshDatabaseSelection: ( + connectionId: string, + options?: { notifyWhenUnchanged?: boolean }, + ) => Promise; getConnectionData: (connectionId: string) => ConnectionData | undefined; isConnectionOpen: (connectionId: string) => boolean; /** Connection ids open in ANY window (shared backend registry). */ diff --git a/src/contexts/DatabaseProvider.tsx b/src/contexts/DatabaseProvider.tsx index 28240ecb..ea0e5813 100644 --- a/src/contexts/DatabaseProvider.tsx +++ b/src/contexts/DatabaseProvider.tsx @@ -2,6 +2,7 @@ import { useState, useCallback, useEffect, useRef, useMemo } from 'react'; import { useTranslation } from 'react-i18next'; import { invoke } from '@tauri-apps/api/core'; import { listen } from '@tauri-apps/api/event'; +import { getCurrentWindow } from '@tauri-apps/api/window'; import { DatabaseContext, type TableInfo, @@ -22,6 +23,9 @@ import { useToast } from '../hooks/useToast'; import { findConnectionsForDrivers } from '../utils/connectionManager'; import { isMultiDatabaseCapable, getEffectiveDatabase, getDatabaseList, reconcileDatabaseSelection } from '../utils/database'; +/** Label of the main window; Tauri defaults to this when none is configured. */ +const MAIN_WINDOW_LABEL = 'main'; + const createEmptyConnectionData = (driver: string = '', name: string = '', dbName: string = ''): ConnectionData => ({ driver, capabilities: null, @@ -133,10 +137,18 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { })); }, []); - // Manually re-runs the same reconciliation that happens on connect (#518), - // for a connection that's already open. Lets a user recover from a database - // dropped mid-session without having to disconnect and reconnect. - const refreshDatabaseSelection = useCallback(async (connectionId: string) => { + // Re-runs the same reconciliation that happens on connect (#518), for a + // connection that's already open. Lets a user recover from a database dropped + // mid-session without having to disconnect and reconnect. + // + // `notifyWhenUnchanged` tells the user the list was already up to date. That + // closes the loop for someone who pressed refresh, but it is noise when the + // reconciliation was triggered automatically (#525) — nobody asked, so + // silence is the right answer when there is nothing to report. + const refreshDatabaseSelection = useCallback(async ( + connectionId: string, + { notifyWhenUnchanged = true }: { notifyWhenUnchanged?: boolean } = {}, + ) => { const conn = connections.find(c => c.id === connectionId); const current = connectionDataMap[connectionId]?.selectedDatabases ?? []; if (!conn || current.length === 0) return; @@ -175,7 +187,9 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { message: `Connection "${conn.name}": removed ${removed.join(', ')} from the database selection (no longer on the server)`, }).catch(() => {}); } else { - showToast(t('sidebar.databaseListUpToDate'), { kind: 'info' }); + if (notifyWhenUnchanged) { + showToast(t('sidebar.databaseListUpToDate'), { kind: 'info' }); + } } } catch (e) { console.error('Failed to refresh database selection:', e); @@ -183,6 +197,13 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { } }, [connections, connectionDataMap, updateConnectionData, showToast, t]); + // Lets the listener below subscribe once: `refreshDatabaseSelection` is + // recreated whenever connection state changes, so depending on it directly + // would tear the subscription down and rebuild it constantly, with a window + // in between where an event would be missed. + const refreshDatabaseSelectionRef = useRef(refreshDatabaseSelection); + refreshDatabaseSelectionRef.current = refreshDatabaseSelection; + const refreshTables = async (targetConnectionId?: string) => { const connId = targetConnectionId ?? activeConnectionId; if (!connId) return; @@ -992,6 +1013,30 @@ export const DatabaseProvider = ({ children }: { children: ReactNode }) => { return () => { unlisten.then(fn => fn()); }; }, []); + // A `DROP DATABASE` that succeeded inside the app leaves the stored selection + // stale; the backend now says so instead of making the user press refresh + // (#525). + // + // Only the main window acts on it. `emit` broadcasts to every window and each + // one mounts its own provider, but reconciling persists through + // `set_selected_databases`, which reloads and rewrites the whole connections + // file — so letting several windows react risks one overwriting another's + // changes. The main window is the stable anchor: dedicated connection windows + // close with their connection. Other windows pick the change up on their next + // refresh. + useEffect(() => { + if (getCurrentWindow().label !== MAIN_WINDOW_LABEL) return; + const unlisten = listen<{ connectionId: string; database: string }>( + 'database-dropped', + (event) => { + void refreshDatabaseSelectionRef.current(event.payload.connectionId, { + notifyWhenUnchanged: false, + }); + }, + ); + return () => { unlisten.then(fn => fn()); }; + }, []); + // Connection Group methods const createGroup = useCallback(async ( name: string,