diff --git a/datasette/app.py b/datasette/app.py index 4ba5d20f..987c77df 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1565,6 +1565,10 @@ class Datasette: conn.execute("SELECT load_extension(?)", [extension]) if self.setting("cache_size_kb"): conn.execute(f"PRAGMA cache_size=-{self.setting('cache_size_kb')}") + # Consistent trigger semantics on every connection - sqlite-utils + # would otherwise enable this on just the write connection, as a + # side effect of the first sqlite-utils based write + conn.execute("PRAGMA recursive_triggers=on") # pylint: disable=no-member if database != INTERNAL_DB_NAME: pm.hook.prepare_connection(conn=conn, database=database, datasette=self) diff --git a/datasette/database.py b/datasette/database.py index 82522e6a..6c1b3a9e 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -757,7 +757,7 @@ class Database: def column_details(conn): # Returns {column_name: (type, is_unique)} - db = sqlite_utils.Database(conn) + db = sqlite_utils.Database(conn, execute_plugins=False) columns = db[table].columns_dict indexes = db[table].indexes details = {} diff --git a/datasette/views/row.py b/datasette/views/row.py index c90a3bbe..feb67eb5 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -749,7 +749,9 @@ class RowDeleteView(BaseView): # Delete table def delete_row(conn): - sqlite_utils.Database(conn)[resolved.table].delete(resolved.pk_values) + sqlite_utils.Database(conn, execute_plugins=False)[resolved.table].delete( + resolved.pk_values + ) try: await resolved.db.execute_write_fn(delete_row, request=request) @@ -830,7 +832,7 @@ class RowUpdateView(BaseView): return Response.error(["Permission denied for alter-table"], 403) def update_row(conn): - sqlite_utils.Database(conn)[resolved.table].update( + sqlite_utils.Database(conn, execute_plugins=False)[resolved.table].update( resolved.pk_values, update, alter=alter ) diff --git a/datasette/views/table.py b/datasette/views/table.py index f7edd744..35afd223 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -1143,7 +1143,7 @@ class TableInsertView(BaseView): row_pk_values_for_later = [tuple(row[pk] for pk in pks) for row in rows] def insert_or_upsert_rows(conn): - table = sqlite_utils.Database(conn)[table_name] + table = sqlite_utils.Database(conn, execute_plugins=False)[table_name] kwargs = {} if upsert: kwargs = { @@ -1407,7 +1407,7 @@ class TableDropView(BaseView): # Drop table def drop_table(conn): - sqlite_utils.Database(conn)[table_name].drop() + sqlite_utils.Database(conn, execute_plugins=False)[table_name].drop() await db.execute_write_fn(drop_table, request=request) await self.ds.track_event( diff --git a/datasette/views/table_create_alter.py b/datasette/views/table_create_alter.py index f3e1779f..35f7ab86 100644 --- a/datasette/views/table_create_alter.py +++ b/datasette/views/table_create_alter.py @@ -889,11 +889,13 @@ class TableCreateView(BaseView): initial_schema = None if table_exists: initial_schema = await db.execute_fn( - lambda conn: sqlite_utils.Database(conn)[table_name].schema + lambda conn: sqlite_utils.Database(conn, execute_plugins=False)[ + table_name + ].schema ) def create_table(conn): - db_for_write = sqlite_utils.Database(conn) + db_for_write = sqlite_utils.Database(conn, execute_plugins=False) table = db_for_write[table_name] if rows: table.insert_all( @@ -1187,7 +1189,9 @@ class TableAlterView(BaseView): before_schema = _table_schema_from_conn(conn, table_name) def apply_operations(operation_conn): - db_for_write = sqlite_utils.Database(operation_conn) + db_for_write = sqlite_utils.Database( + operation_conn, execute_plugins=False + ) table = db_for_write[table_name] current_table_name = table_name diff --git a/docs/changelog.rst b/docs/changelog.rst index 8cb72005..caa1050f 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -14,6 +14,7 @@ Unreleased - ``await db.execute_write_script()`` is now transactional, matching its documentation: if any statement in the script fails, none of its statements are applied. Scripts containing statements that cannot run inside a transaction, or that manage transactions themselves, fall back to the previous ``conn.executescript()`` autocommit behavior. (:issue:`2831`) - The JSON write API is now atomic per request: ``/db/-/create`` with initial rows, multi-operation ``/db/table/-/alter`` calls and inserts using ``"return": true`` now either fully apply or roll back entirely if any part fails. Previously a failure part way through could leave earlier writes from the same request permanently committed. (:issue:`2831`) - Rebuilding the internal database catalog for a database is now a single atomic write. Previously the rebuild used six separate transactions, so queries against the internal database could observe a database with missing catalog rows while a rebuild was in progress. (:issue:`2831`) +- sqlite-utils plugins no longer have their ``prepare_connection()`` hooks executed against Datasette's database connections - use Datasette's own :ref:`prepare_connection() ` plugin hook to customize connections. ``PRAGMA recursive_triggers=on`` is now applied consistently to every connection Datasette opens - previously it was enabled just on the write connection, as a side effect of the first sqlite-utils based write. (:issue:`2831`) .. _v1_0_a36: diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 691f7ac9..4218712e 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -2782,3 +2782,38 @@ async def test_insert_with_return_failing_row_is_atomic(ds_write): await ds_write.get_database("data").execute("select count(*) from docs") ).single_value() assert count == 0 + + +@pytest.mark.asyncio +async def test_write_api_does_not_run_sqlite_utils_plugins(ds_write): + # https://github.com/simonw/datasette/issues/2831 + # sqlite-utils plugins should not have their prepare_connection hooks + # executed against Datasette's connections + import sqlite_utils.plugins + from sqlite_utils import hookimpl + + prepared = [] + + class TrackingPlugin: + @hookimpl + def prepare_connection(self, conn): + prepared.append(conn) + + sqlite_utils.plugins.pm.register(TrackingPlugin(), name="datasette-test-tracking") + try: + token = write_token(ds_write) + response = await ds_write.client.post( + "/data/docs/-/insert", + json={"rows": [{"id": 1, "title": "one"}]}, + headers=_headers(token), + ) + assert response.status_code == 201 + response = await ds_write.client.post( + "/data/docs/1/-/update", + json={"update": {"title": "two"}}, + headers=_headers(token), + ) + assert response.status_code == 200 + assert prepared == [] + finally: + sqlite_utils.plugins.pm.unregister(name="datasette-test-tracking") diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index 74818df9..fb1adcba 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -1302,3 +1302,24 @@ 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_recursive_triggers_enabled_on_all_connections(tmp_path): + # https://github.com/simonw/datasette/issues/2831 + # Previously recursive_triggers was only enabled on the write connection, + # and only as a side effect of the first sqlite-utils based write - so + # trigger semantics could differ between connections + path = str(tmp_path / "test.db") + sqlite3.connect(path).close() + datasette = Datasette([path]) + db = datasette.get_database("test") + write_value = await db.execute_write_fn( + lambda conn: conn.execute("PRAGMA recursive_triggers").fetchone()[0], + transaction=False, + ) + read_value = await db.execute_fn( + lambda conn: conn.execute("PRAGMA recursive_triggers").fetchone()[0] + ) + assert write_value == 1 + assert read_value == 1