From e34aac77a9ccd56efec04752fef07b5fd74c82d6 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 19 May 2026 02:07:18 +0000 Subject: [PATCH 001/380] Add Database.request_connection() for per-request scoped connections Opens a fresh sqlite3 connection scoped to an async-with block, intended for short-lived per-request work such as installing a set_authorizer callback derived from request.actor before running queries. The connection is not pooled, not cached, and closed when the context exits. - Database.request_connection(write=False, run_prepare_connection_hook=False) yields a RequestConnection with .connection, .execute() and .execute_fn() - Datasette._prepare_connection gains run_plugin_hook=True kwarg; the prepare_connection plugin hook is skipped by default for these connections to keep them cheap - Database.connect gains track=False to skip _all_file_connections tracking when the caller owns lifecycle - Database.execute body extracted into _make_sql_operation so both pooled execute() and RequestConnection.execute() share it https://claude.ai/code/session_01GdaNscbub6d2MPaUANrhJj --- datasette/app.py | 4 +- datasette/database.py | 112 +++++++++++++++++++--- tests/test_internals_database.py | 153 +++++++++++++++++++++++++++++++ 3 files changed, 255 insertions(+), 14 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 358081ef..f141ca68 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1229,7 +1229,7 @@ class Datasette: if query: return query - def _prepare_connection(self, conn, database): + def _prepare_connection(self, conn, database, *, run_plugin_hook=True): conn.row_factory = sqlite3.Row conn.text_factory = lambda x: str(x, "utf-8", "replace") if self.sqlite_extensions and database != INTERNAL_DB_NAME: @@ -1245,7 +1245,7 @@ class Datasette: if self.setting("cache_size_kb"): conn.execute(f"PRAGMA cache_size=-{self.setting('cache_size_kb')}") # pylint: disable=no-member - if database != INTERNAL_DB_NAME: + if database != INTERNAL_DB_NAME and run_plugin_hook: pm.hook.prepare_connection(conn=conn, database=database, datasette=self) # If self.crossdb and this is _memory, connect the first SQLITE_LIMIT_ATTACHED databases if self.crossdb and database == "_memory": diff --git a/datasette/database.py b/datasette/database.py index 66d50ffa..21d62a86 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -1,6 +1,7 @@ import asyncio import atexit from collections import namedtuple +from contextlib import asynccontextmanager import inspect import os from pathlib import Path @@ -131,7 +132,7 @@ class Database: else: return "db" - def connect(self, write=False): + def connect(self, write=False, track=True): extra_kwargs = {} if write: extra_kwargs["isolation_level"] = "IMMEDIATE" @@ -161,7 +162,8 @@ class Database: conn = sqlite3.connect( f"file:{self.path}{qs}", uri=True, check_same_thread=False, **extra_kwargs ) - self._all_file_connections.append(conn) + if track: + self._all_file_connections.append(conn) if self.is_temp_disk and not self._wal_enabled: conn.execute("PRAGMA journal_mode=WAL") self._wal_enabled = True @@ -478,17 +480,9 @@ class Database: future.add_done_callback(self._remove_pending_execute_future) return await asyncio.wrap_future(future) - async def execute( - self, - sql, - params=None, - truncate=False, - custom_time_limit=None, - page_size=None, - log_sql_errors=True, + def _make_sql_operation( + self, sql, params, truncate, custom_time_limit, page_size, log_sql_errors ): - """Executes sql against db_name in a thread""" - self._check_not_closed() page_size = page_size or self.ds.page_size def sql_operation_in_thread(conn): @@ -528,10 +522,72 @@ class Database: else: return Results(rows, False, cursor.description) + return sql_operation_in_thread + + async def execute( + self, + sql, + params=None, + truncate=False, + custom_time_limit=None, + page_size=None, + log_sql_errors=True, + ): + """Executes sql against db_name in a thread""" + self._check_not_closed() + sql_operation_in_thread = self._make_sql_operation( + sql, params, truncate, custom_time_limit, page_size, log_sql_errors + ) with trace("sql", database=self.name, sql=sql.strip(), params=params): results = await self.execute_fn(sql_operation_in_thread) return results + async def _execute_fn_on_connection(self, conn, fn): + """Run fn(conn) on the shared executor (or inline in non-threaded mode). + + The caller owns the connection's lifecycle; this method does not + cache or close it. Used by request_connection(). + """ + self._check_not_closed() + if self.ds.executor is None: + return fn(conn) + + def in_thread(): + return fn(conn) + + with self._pending_execute_futures_lock: + self._check_not_closed() + future = self.ds.executor.submit(in_thread) + self._pending_execute_futures.add(future) + future.add_done_callback(self._remove_pending_execute_future) + return await asyncio.wrap_future(future) + + @asynccontextmanager + async def request_connection(self, write=False, run_prepare_connection_hook=False): + """Open a fresh sqlite3 connection scoped to an ``async with`` block. + + Intended for short-lived per-request work — for example, installing + a ``set_authorizer`` callback derived from ``request.actor`` before + running queries. The connection is not added to the pool, not + cached, and is closed when the context exits. + + Pass ``run_prepare_connection_hook=True`` to opt into the + ``prepare_connection`` plugin hook; by default it is skipped so + these connections stay cheap. + """ + self._check_not_closed() + conn = self.connect(write=write, track=False) + self.ds._prepare_connection( + conn, self.name, run_plugin_hook=run_prepare_connection_hook + ) + try: + yield RequestConnection(self, conn) + finally: + try: + conn.close() + except Exception: + pass + @property def hash(self): if self.cached_hash is not None: @@ -931,6 +987,38 @@ def _deliver_write_result(task, result, exception): pass +class RequestConnection: + """Thin async wrapper around a single sqlite3.Connection. + + Yielded by :meth:`Database.request_connection`. Exposes ``execute`` and + ``execute_fn`` with the same semantics as :class:`Database`, but bound + to the underlying ``connection`` so a caller can attach per-request + state (e.g. ``set_authorizer``) without touching the pool. + """ + + def __init__(self, db, connection): + self._db = db + self.connection = connection + + async def execute_fn(self, fn): + return await self._db._execute_fn_on_connection(self.connection, fn) + + async def execute( + self, + sql, + params=None, + truncate=False, + custom_time_limit=None, + page_size=None, + log_sql_errors=True, + ): + fn = self._db._make_sql_operation( + sql, params, truncate, custom_time_limit, page_size, log_sql_errors + ) + with trace("sql", database=self._db.name, sql=sql.strip(), params=params): + return await self._db._execute_fn_on_connection(self.connection, fn) + + class QueryInterrupted(Exception): def __init__(self, e, sql, params): self.e = e diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index 75ae8d39..434c0e37 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -923,3 +923,156 @@ async def test_database_close_is_idempotent(tmpdir): # Second call should be a no-op, not raise db.close() ds._internal_database.close() + + +@pytest.mark.asyncio +async def test_request_connection_basic(db): + async with db.request_connection() as conn: + results = await conn.execute("select 1 + 1") + assert results.single_value() == 2 + + +@pytest.mark.asyncio +async def test_request_connection_exposes_raw_sqlite_connection(db): + async with db.request_connection() as conn: + assert isinstance(conn.connection, sqlite3.Connection) + # Direct execution on the raw connection works too + assert conn.connection.execute("select 1").fetchone()[0] == 1 + + +@pytest.mark.asyncio +async def test_request_connection_fresh_each_call(db): + async with db.request_connection() as conn_a: + async with db.request_connection() as conn_b: + assert conn_a.connection is not conn_b.connection + + +@pytest.mark.asyncio +async def test_request_connection_closes_on_exit(db): + async with db.request_connection() as conn: + raw = conn.connection + await conn.execute("select 1") + with pytest.raises(sqlite3.ProgrammingError): + raw.execute("select 1") + + +@pytest.mark.asyncio +async def test_request_connection_readonly_by_default(db): + async with db.request_connection() as conn: + with pytest.raises(sqlite3.OperationalError): + await conn.execute( + "create table should_not_exist (id integer primary key)" + ) + + +@pytest.mark.asyncio +async def test_request_connection_writable(): + ds = Datasette(memory=True) + db = ds.add_memory_database("test_request_connection_writable") + async with db.request_connection(write=True) as conn: + await conn.execute_fn( + lambda c: c.execute("create table t (id integer primary key)") + ) + await conn.execute_fn(lambda c: c.execute("insert into t (id) values (1)")) + row = await conn.execute_fn( + lambda c: c.execute("select id from t").fetchone() + ) + assert row[0] == 1 + + +@pytest.mark.asyncio +async def test_request_connection_authorizer_does_not_leak_to_pool(db): + def deny_all(action, *args): + return sqlite3.SQLITE_DENY + + async with db.request_connection() as conn: + conn.connection.set_authorizer(deny_all) + with pytest.raises(sqlite3.DatabaseError): + await conn.execute("select 1") + + # Plain pool execute should still succeed + assert (await db.execute("select 1")).single_value() == 1 + + +@pytest.mark.asyncio +async def test_request_connection_skips_prepare_connection_hook_by_default(db): + # The `fixtures` db has my_plugin loaded, which would register + # convert_units via the prepare_connection hook. The per-request + # connection should NOT have that function available by default. + async with db.request_connection() as conn: + with pytest.raises(sqlite3.OperationalError): + await conn.execute("select convert_units(100, 'm', 'ft')") + + +@pytest.mark.asyncio +async def test_request_connection_runs_prepare_connection_hook_when_opted_in(db): + async with db.request_connection(run_prepare_connection_hook=True) as conn: + result = await conn.execute("select convert_units(100, 'm', 'ft')") + assert result.single_value() == pytest.approx(328.0839) + + +@pytest.mark.asyncio +async def test_request_connection_row_factory_applied(db): + async with db.request_connection() as conn: + row = (await conn.execute("select 1 as one")).first() + assert isinstance(row, sqlite3.Row) + assert row["one"] == 1 + + +@pytest.mark.asyncio +async def test_request_connection_not_tracked_in_all_file_connections(db): + before = list(db._all_file_connections) + async with db.request_connection() as conn: + pass + assert db._all_file_connections == before + + +@pytest.mark.asyncio +async def test_request_connection_cleans_up_on_exception(db): + raw = None + with pytest.raises(RuntimeError, match="boom"): + async with db.request_connection() as conn: + raw = conn.connection + raise RuntimeError("boom") + with pytest.raises(sqlite3.ProgrammingError): + raw.execute("select 1") + + +@pytest.mark.asyncio +async def test_request_connection_execute_fn(db): + async with db.request_connection() as conn: + value = await conn.execute_fn( + lambda c: c.execute("select 41 + 1").fetchone()[0] + ) + assert value == 42 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("disable_threads", (False, True)) +async def test_request_connection_non_threaded(disable_threads): + if disable_threads: + ds = Datasette(memory=True, settings={"num_sql_threads": 0}) + else: + ds = Datasette(memory=True) + db = ds.add_memory_database("test_request_connection_non_threaded") + async with db.request_connection() as conn: + assert (await conn.execute("select 1")).single_value() == 1 + assert ( + await conn.execute_fn(lambda c: c.execute("select 2").fetchone()[0]) + ) == 2 + + +@pytest.mark.asyncio +async def test_request_connection_raises_after_database_closed(tmpdir): + path = str(tmpdir / "closed_req.db") + conn = sqlite3.connect(path) + conn.execute("create table t (id integer primary key)") + conn.close() + ds = Datasette([path]) + db = ds.get_database("closed_req") + await db.execute("select 1") + db.close() + with pytest.raises(DatasetteClosedError): + async with db.request_connection() as conn: + pass + ds._internal_database.close() From 7a914f8c656de2ffa3f662e49bc95b24dd36b854 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 20 May 2026 12:14:50 -0700 Subject: [PATCH 002/380] Clear stale tables/other resources when DB removed, closes #2723 --- datasette/app.py | 23 +++++++++++++++---- docs/changelog.rst | 2 +- tests/test_internal_db.py | 48 +++++++++++++++++++++++++++++++++++++++ 3 files changed, 67 insertions(+), 6 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 358081ef..218d40c6 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -618,11 +618,24 @@ class Datasette: stale_databases = set(current_schema_versions.keys()) - set( self.databases.keys() ) - for stale_db_name in stale_databases: - await internal_db.execute_write( - "DELETE FROM catalog_databases WHERE database_name = ?", - [stale_db_name], - ) + if stale_databases: + + def delete_stale_database_catalog(conn): + for stale_db_name in stale_databases: + for table in ( + "catalog_columns", + "catalog_foreign_keys", + "catalog_indexes", + "catalog_views", + "catalog_tables", + "catalog_databases", + ): + conn.execute( + "DELETE FROM {} WHERE database_name = ?".format(table), + [stale_db_name], + ) + + await internal_db.execute_write_fn(delete_stale_database_catalog) for database_name, db in self.databases.items(): schema_version = (await db.execute("PRAGMA schema_version")).first()[0] # Compare schema versions to see if we should skip it diff --git a/docs/changelog.rst b/docs/changelog.rst index 5b637797..eb408287 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -10,7 +10,7 @@ Unreleased ---------- - Dropped Janus as a dependency, previously used to manage the write queue. This should not have any impact on plugin developers or end-users. (:issue:`1752`) - +- Fixed a bug where stale tables and other related resources were not removed from ``catalog_*`` tables when a database was removed. (:issue:`2723`) .. _v1_0_a29: diff --git a/tests/test_internal_db.py b/tests/test_internal_db.py index 7a0d1630..ec013b43 100644 --- a/tests/test_internal_db.py +++ b/tests/test_internal_db.py @@ -139,3 +139,51 @@ async def test_stale_catalog_entry_database_fix(tmp_path): f"Index page should return 200, not {response.status_code}. " "This fails due to stale catalog entries causing KeyError." ) + + +@pytest.mark.asyncio +async def test_stale_catalog_child_entries_removed_for_missing_database(tmp_path): + from datasette.app import Datasette + + import sqlite3 + + internal_db_path = str(tmp_path / "internal.db") + alpha_db_path = str(tmp_path / "alpha.db") + bravo_db_path = str(tmp_path / "bravo.db") + + for db_path, table_name in ( + (alpha_db_path, "alpha_table"), + (bravo_db_path, "bravo_table"), + (bravo_db_path, "bravo_table_2"), + ): + conn = sqlite3.connect(db_path) + conn.execute(f"CREATE TABLE {table_name} (id INTEGER PRIMARY KEY)") + conn.close() + + ds1 = Datasette(files=[alpha_db_path, bravo_db_path], internal=internal_db_path) + await ds1.invoke_startup() + + catalog_tables = await ds1.get_internal_database().execute(""" + SELECT database_name, table_name + FROM catalog_tables + ORDER BY database_name, table_name + """) + assert [tuple(row) for row in catalog_tables.rows] == [ + ("alpha", "alpha_table"), + ("bravo", "bravo_table"), + ("bravo", "bravo_table_2"), + ] + + ds1.close() + + ds2 = Datasette(files=[alpha_db_path], internal=internal_db_path) + await ds2.invoke_startup() + + catalog_tables = await ds2.get_internal_database().execute(""" + SELECT database_name, table_name + FROM catalog_tables + ORDER BY database_name, table_name + """) + assert [tuple(row) for row in catalog_tables.rows] == [("alpha", "alpha_table")] + + ds2.close() From 5d6de0154d18d0ed07e7ac7cf02cd3c37324ddbc Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 20 May 2026 12:18:01 -0700 Subject: [PATCH 003/380] Bump Black to black==26.3.1 Refs https://github.com/advisories/GHSA-3936-cmfr-pm3m --- pyproject.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pyproject.toml b/pyproject.toml index c50c720a..38085476 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -62,7 +62,7 @@ dev = [ "pytest-xdist>=2.2.1", "pytest-asyncio>=1.2.0", "beautifulsoup4>=4.8.1", - "black==26.1.0", + "black==26.3.1", "blacken-docs==1.20.0", "pytest-timeout>=1.4.2", "trustme>=0.7", From bbbc1cd59620c7c53a2eff6138ef338c8901ba6e Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 20 May 2026 12:33:33 -0700 Subject: [PATCH 004/380] Remove height: 100% to fix Safari bug, closes #2724 --- datasette/static/navigation-search.js | 1 - docs/changelog.rst | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/datasette/static/navigation-search.js b/datasette/static/navigation-search.js index 95e7dfc5..d2c300e2 100644 --- a/datasette/static/navigation-search.js +++ b/datasette/static/navigation-search.js @@ -54,7 +54,6 @@ class NavigationSearch extends HTMLElement { .search-container { display: flex; flex-direction: column; - height: 100%; } .search-input-wrapper { diff --git a/docs/changelog.rst b/docs/changelog.rst index eb408287..56c49ea3 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -11,6 +11,7 @@ Unreleased - Dropped Janus as a dependency, previously used to manage the write queue. This should not have any impact on plugin developers or end-users. (:issue:`1752`) - Fixed a bug where stale tables and other related resources were not removed from ``catalog_*`` tables when a database was removed. (:issue:`2723`) +- Fixed a Safari bug with the table search mechanism triggered by pressing ``/``. (:issue:`2724`) .. _v1_0_a29: From 54b272baf61cb014e6d262fea1b01dc84981c1d0 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 20 May 2026 12:39:54 -0700 Subject: [PATCH 005/380] Remove existing stale catalog_ tables, refs #2723 Now if there are any existing stale records in internal.db those will be removed as well. --- datasette/app.py | 30 ++++++++++++++++---------- tests/test_internal_db.py | 45 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 11 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 218d40c6..b1f9b2f7 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -614,22 +614,30 @@ class Datasette: "select database_name, schema_version from catalog_databases" ) } - # Delete stale entries for databases that are no longer attached - stale_databases = set(current_schema_versions.keys()) - set( - self.databases.keys() + catalog_table_names = ( + "catalog_columns", + "catalog_foreign_keys", + "catalog_indexes", + "catalog_views", + "catalog_tables", + "catalog_databases", ) + # Delete stale entries for databases that are no longer attached + catalog_database_names = set(current_schema_versions.keys()) + for table in catalog_table_names[:-1]: + catalog_database_names.update( + row["database_name"] + for row in await internal_db.execute( + "select distinct database_name from {}".format(table) + ) + if row["database_name"] is not None + ) + stale_databases = catalog_database_names - set(self.databases.keys()) if stale_databases: def delete_stale_database_catalog(conn): for stale_db_name in stale_databases: - for table in ( - "catalog_columns", - "catalog_foreign_keys", - "catalog_indexes", - "catalog_views", - "catalog_tables", - "catalog_databases", - ): + for table in catalog_table_names: conn.execute( "DELETE FROM {} WHERE database_name = ?".format(table), [stale_db_name], diff --git a/tests/test_internal_db.py b/tests/test_internal_db.py index ec013b43..dcf14126 100644 --- a/tests/test_internal_db.py +++ b/tests/test_internal_db.py @@ -187,3 +187,48 @@ async def test_stale_catalog_child_entries_removed_for_missing_database(tmp_path assert [tuple(row) for row in catalog_tables.rows] == [("alpha", "alpha_table")] ds2.close() + + +@pytest.mark.asyncio +async def test_orphan_stale_catalog_child_entries_removed(tmp_path): + from datasette.app import Datasette + + import sqlite3 + + internal_db_path = str(tmp_path / "internal.db") + alpha_db_path = str(tmp_path / "alpha.db") + + conn = sqlite3.connect(alpha_db_path) + conn.execute("CREATE TABLE alpha_table (id INTEGER PRIMARY KEY)") + conn.close() + + ds1 = Datasette(files=[alpha_db_path], internal=internal_db_path) + await ds1.invoke_startup() + ds1.close() + + # Simulate the state left behind by old cleanup code: the parent database + # row was deleted, but child catalog rows survived because foreign key + # enforcement is not enabled for these internal catalog writes. + conn = sqlite3.connect(internal_db_path) + conn.execute("DELETE FROM catalog_databases WHERE database_name = 'fixtures'") + conn.execute(""" + INSERT INTO catalog_tables (database_name, table_name, rootpage, sql) + VALUES ('fixtures', 'stale_table', 1, 'CREATE TABLE stale_table (id INTEGER)') + """) + conn.commit() + conn.close() + + ds2 = Datasette(files=[alpha_db_path], internal=internal_db_path) + await ds2.invoke_startup() + + catalog_tables = await ds2.get_internal_database().execute(""" + SELECT database_name, table_name + FROM catalog_tables + ORDER BY database_name, table_name + """) + assert [tuple(row) for row in catalog_tables.rows] == [("alpha", "alpha_table")] + + response = await ds2.client.get("/-/tables.json") + assert response.status_code == 200 + + ds2.close() From d3330695fa42ad1cdb2f2b1b80470e95bea8ed12 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 20 May 2026 13:23:05 -0700 Subject: [PATCH 006/380] Always show 'Jump to...' menu item, closes #2725 --- datasette/static/app.css | 26 ++++++++++++++++++++++++++ datasette/static/navigation-search.js | 13 ++++++++++++- datasette/templates/base.html | 7 +++---- docs/changelog.rst | 1 + tests/test_html.py | 18 +++++++++++++++--- 5 files changed, 57 insertions(+), 8 deletions(-) diff --git a/datasette/static/app.css b/datasette/static/app.css index 1ce84bc8..c21d0dc4 100644 --- a/datasette/static/app.css +++ b/datasette/static/app.css @@ -362,6 +362,32 @@ form.nav-menu-logout { .nav-menu-inner a { display: block; } +.nav-menu-inner button.button-as-link { + display: block; + width: 100%; + text-align: left; + font: inherit; +} +.nav-menu-inner .keyboard-shortcut { + float: right; + box-sizing: border-box; + min-width: 1.4em; + margin-left: 0.75rem; + padding: 0 0.35em; + border: 1px solid rgba(255,255,244,0.6); + border-radius: 3px; + background: rgba(255,255,244,0.12); + font-family: ui-monospace, SFMono-Regular, Menlo, Consolas, monospace; + font-size: 0.85em; + line-height: 1.35; + text-align: center; + text-decoration: none; +} +@media (max-width: 640px) { + .nav-menu-inner .keyboard-shortcut { + display: none; + } +} /* Table/database actions menu */ .page-action-menu { diff --git a/datasette/static/navigation-search.js b/datasette/static/navigation-search.js index d2c300e2..09d58898 100644 --- a/datasette/static/navigation-search.js +++ b/datasette/static/navigation-search.js @@ -199,6 +199,15 @@ class NavigationSearch extends HTMLElement { } }); + document.addEventListener("click", (e) => { + const trigger = e.target.closest("[data-navigation-search-open]"); + if (trigger) { + e.preventDefault(); + trigger.closest("details")?.removeAttribute("open"); + this.openMenu(); + } + }); + // Input event input.addEventListener("input", (e) => { this.handleSearch(e.target.value); @@ -390,7 +399,9 @@ class NavigationSearch extends HTMLElement { const dialog = this.shadowRoot.querySelector("dialog"); const input = this.shadowRoot.querySelector(".search-input"); - dialog.showModal(); + if (!dialog.open) { + dialog.showModal(); + } input.value = ""; input.focus(); diff --git a/datasette/templates/base.html b/datasette/templates/base.html index 21f8c693..b4fecf70 100644 --- a/datasette/templates/base.html +++ b/datasette/templates/base.html @@ -20,7 +20,7 @@