diff --git a/datasette/app.py b/datasette/app.py index 4ba5d20f..c44f9095 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -2575,7 +2575,7 @@ class Datasette: JsonDataView.as_view( self, "plugins.json", - lambda request: {"plugins": self._plugins(request)}, + self._plugins, needs_request=True, ), r"/-/plugins(\.(?Pjson))?$", diff --git a/datasette/database.py b/datasette/database.py index e7fe1ed9..eb402b0c 100644 --- a/datasette/database.py +++ b/datasette/database.py @@ -246,6 +246,7 @@ class Database: request=None, return_all=False, returning_limit=EXECUTE_WRITE_RETURNING_LIMIT, + transaction=True, ): self._check_not_closed() if returning_limit < 0: @@ -258,7 +259,9 @@ class Database: ) with trace("sql", database=self.name, sql=sql.strip(), params=params): - results = await self.execute_write_fn(_inner, block=block, request=request) + results = await self.execute_write_fn( + _inner, block=block, request=request, transaction=transaction + ) return results async def execute_write_script(self, sql, block=True, request=None): @@ -348,6 +351,7 @@ class Database: self.ds._prepare_connection(self._write_connection, self.name) if transaction: with self._write_connection: + self._write_connection.execute("BEGIN IMMEDIATE") result = fn(self._write_connection) else: result = fn(self._write_connection) @@ -477,6 +481,7 @@ class Database: try: if task.transaction: with conn: + conn.execute("BEGIN IMMEDIATE") result = task.fn(conn) else: result = task.fn(conn) diff --git a/datasette/views/database.py b/datasette/views/database.py index 10dc66ae..11646f45 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -643,8 +643,15 @@ class QueryView(View): ok = None redirect_url = None try: + execute_write_kwargs = {"request": request} + if stored_query.is_trusted: + analysis = await db.analyze_sql(stored_query.sql, params_for_query) + if any( + operation.operation == "vacuum" for operation in analysis.operations + ): + execute_write_kwargs["transaction"] = False cursor = await db.execute_write( - stored_query.sql, params_for_query, request=request + stored_query.sql, params_for_query, **execute_write_kwargs ) # success message can come from on_success_message or on_success_message_sql message = None diff --git a/docs/internals.rst b/docs/internals.rst index 2048c7e4..d2bd46ef 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -2023,8 +2023,8 @@ Example usage: .. _database_execute_write: -await db.execute_write(sql, params=None, block=True, request=None, return_all=False, returning_limit=10) --------------------------------------------------------------------------------------------------------- +await db.execute_write(sql, params=None, block=True, request=None, return_all=False, returning_limit=10, transaction=True) +-------------------------------------------------------------------------------------------------------------------------- SQLite only allows one database connection to write at a time. Datasette handles this for you by maintaining a queue of writes to be executed against a given database. Plugins can submit write operations to this queue and they will be executed in the order in which they are received. @@ -2059,7 +2059,9 @@ If you need to retrieve every row returned by a statement, pass ``return_all=Tru If you pass ``block=False`` this behavior changes to "fire and forget" - queries will be added to the write queue and executed in a separate thread while your code can continue to do other things. The method will return a UUID representing the queued task. -Each call to ``execute_write()`` will be executed inside a transaction. +Each call to ``execute_write()`` will be executed inside a transaction. Pass +``transaction=False`` for statements such as ``VACUUM`` that cannot run inside +a transaction. .. _database_execute_write_script: diff --git a/docs/introspection.rst b/docs/introspection.rst index b78e4860..14b6249f 100644 --- a/docs/introspection.rst +++ b/docs/introspection.rst @@ -80,18 +80,15 @@ Shows a list of currently installed plugins and their versions. `Plugins example .. code-block:: json - { - "ok": true, - "plugins": [ - { - "name": "datasette_cluster_map", - "static": true, - "templates": false, - "version": "0.10", - "hooks": ["extra_css_urls", "extra_js_urls", "extra_body_script"] - } - ] - } + [ + { + "name": "datasette_cluster_map", + "static": true, + "templates": false, + "version": "0.10", + "hooks": ["extra_css_urls", "extra_js_urls", "extra_body_script"] + } + ] Add ``?all=1`` to include details of the default plugins baked into Datasette. diff --git a/tests/test_api.py b/tests/test_api.py index 5ed14283..d5f519b9 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -614,13 +614,13 @@ async def test_plugins_json(ds_client): response = await ds_client.get("/-/plugins.json") # Filter out TrackEventPlugin actual_plugins = sorted( - [p for p in response.json()["plugins"] if p["name"] != "TrackEventPlugin"], + [p for p in response.json() if p["name"] != "TrackEventPlugin"], key=lambda p: p["name"], ) assert EXPECTED_PLUGINS == actual_plugins # Try with ?all=1 response = await ds_client.get("/-/plugins.json?all=1") - names = {p["name"] for p in response.json()["plugins"]} + names = {p["name"] for p in response.json()} assert names.issuperset(p["name"] for p in EXPECTED_PLUGINS) assert names.issuperset(DEFAULT_PLUGINS) diff --git a/tests/test_config_dir.py b/tests/test_config_dir.py index 42c6ae60..636b17eb 100644 --- a/tests/test_config_dir.py +++ b/tests/test_config_dir.py @@ -109,7 +109,7 @@ def test_settings(config_dir_client): def test_plugins(config_dir_client): response = config_dir_client.get("/-/plugins.json") assert 200 == response.status - plugins = response.json["plugins"] + plugins = response.json assert "hooray.py" in {p["name"] for p in plugins} assert "non_py_file.txt" not in {p["name"] for p in plugins} assert "mypy_cache" not in {p["name"] for p in plugins} diff --git a/tests/test_internals_database.py b/tests/test_internals_database.py index bad4e8ca..b1a212d9 100644 --- a/tests/test_internals_database.py +++ b/tests/test_internals_database.py @@ -11,6 +11,7 @@ from datasette.database import _deliver_write_result from datasette.utils.sqlite import sqlite3, supports_returning from datasette.utils import Column import pytest +import sqlite_utils import time import uuid @@ -718,6 +719,41 @@ async def test_execute_write_fn_exception(db): await db.execute_write_fn(write_fn) +@pytest.mark.asyncio +@pytest.mark.parametrize("num_sql_threads", (0, 1)) +async def test_execute_write_fn_sqlite_utils_transaction(tmp_path, num_sql_threads): + # A write inside a failing Datasette task must never become visible or + # survive the rollback. Exercise both the synchronous and writer-thread + # paths against a file-backed database so a second connection can observe + # committed state independently. + db_path = tmp_path / "test.db" + sqlite3.connect(db_path).close() + ds = Datasette([str(db_path)], settings={"num_sql_threads": num_sql_threads}) + db = ds.get_database("test") + await db.execute_write("create table items (id integer primary key)") + # This reader is used inside the write callback, which may run on another + # thread, but it is never accessed concurrently. + reader = sqlite3.connect(db_path, check_same_thread=False) + + def insert_then_fail(conn): + # Datasette must open the outer transaction before sqlite-utils writes. + assert conn.in_transaction + sqlite_utils.Database(conn)["items"].insert({"id": 1}) + # If sqlite-utils committed its own transaction, this would return 1. + assert reader.execute("select count(*) from items").fetchone()[0] == 0 + # Simulate a later step failing after the sqlite-utils write succeeded. + raise ValueError("deliberate") + + try: + with pytest.raises(ValueError, match="deliberate"): + await db.execute_write_fn(insert_then_fail) + # The outer transaction must roll back the sqlite-utils write as well. + assert reader.execute("select count(*) from items").fetchone()[0] == 0 + finally: + reader.close() + db.close() + + @pytest.mark.asyncio @pytest.mark.parametrize("param_name", ["conn", "connection", "db", "c"]) async def test_execute_write_fn_accepts_any_single_param_name(db, param_name): diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 5c4034db..59b1c0bf 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -1482,7 +1482,7 @@ async def test_plugin_is_installed(): datasette.pm.register(DummyPlugin(), name="DummyPlugin") response = await datasette.client.get("/-/plugins.json") assert response.status_code == 200 - installed_plugins = {p["name"] for p in response.json()["plugins"]} + installed_plugins = {p["name"] for p in response.json()} assert "DummyPlugin" in installed_plugins finally: diff --git a/tests/test_success_envelope.py b/tests/test_success_envelope.py index 3c413d73..46a78ef0 100644 --- a/tests/test_success_envelope.py +++ b/tests/test_success_envelope.py @@ -2,8 +2,8 @@ Tests for the canonical JSON success envelope. Every JSON object returned by a Datasette endpoint on success should include -"ok": true. (Endpoints that return a top-level array are being converted to -objects separately - see /-/plugins, /-/databases, /-/actions.) +"ok": true. /-/plugins intentionally returns a top-level array instead, while +/-/databases and /-/actions use the object envelope. """ import pytest @@ -79,17 +79,16 @@ async def test_permissions_post_success_has_ok_true(ds_envelope): @pytest.mark.asyncio -async def test_plugins_json_is_object(ds_client): +async def test_plugins_json_is_array(ds_client): response = await ds_client.get("/-/plugins.json") assert response.status_code == 200 data = response.json() - assert set(data.keys()) == {"ok", "plugins"} - assert data["ok"] is True - assert isinstance(data["plugins"], list) + assert isinstance(data, list) + assert all(isinstance(plugin, dict) for plugin in data) # ?all=1 should include Datasette's default plugins in the same shape response_all = await ds_client.get("/-/plugins.json?all=1") - all_plugins = response_all.json()["plugins"] - assert len(all_plugins) > len(data["plugins"]) + all_plugins = response_all.json() + assert len(all_plugins) > len(data) @pytest.mark.asyncio