diff --git a/datasette/app.py b/datasette/app.py index c82ea0759f..b3c9664281 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -45,7 +45,7 @@ from .column_types import SQLiteType from .csrf import CrossOriginProtectionMiddleware from .database import Database, QueryInterrupted -from .events import Event +from .events import AddDatabaseEvent, Event, RemoveDatabaseEvent from .plugins import DEFAULT_PLUGINS, get_plugins, pm from .renderer import json_renderer from .resources import DatabaseResource, TableResource @@ -423,6 +423,9 @@ def __init__( ): self._startup_invoked = False self._closed = False + # Strong references to in-flight fire-and-forget event dispatch + # tasks, so they cannot be garbage-collected before completing + self._pending_event_tasks = set() assert config_dir is None or isinstance( config_dir, Path ), "config_dir= should be a pathlib.Path" @@ -935,6 +938,14 @@ def add_database(self, db, name=None, route=None): new_databases[name] = db # don't mutate! that causes race conditions with live import self.databases = new_databases + self._track_event_soon( + AddDatabaseEvent( + actor=None, + database=db.name, + path=str(Path(db.path).resolve()) if db.path else None, + is_memory=db.is_memory, + ) + ) return db def add_memory_database(self, memory_name, name=None, route=None): @@ -943,10 +954,41 @@ def add_memory_database(self, memory_name, name=None, route=None): ) def remove_database(self, name): - self.get_database(name).close() + db = self.get_database(name) + # Capture event details before close() - is_temp_disk databases + # delete their backing file during close() + path = str(Path(db.path).resolve()) if db.path else None + is_memory = db.is_memory + # Fire the event only after close() returns: close() drains any + # queued writes, so listeners doing a final read of the file see + # everything + db.close() new_databases = self.databases.copy() new_databases.pop(name) self.databases = new_databases + self._track_event_soon( + RemoveDatabaseEvent( + actor=None, + database=name, + path=path, + is_memory=is_memory, + ) + ) + + def _track_event_soon(self, event): + # Best-effort fire-and-forget event dispatch from synchronous code. + # If startup has not run (event classes are not yet registered) or + # there is no running event loop, the event is intentionally + # dropped - lifecycle events are documented as runtime-only + if not self._startup_invoked: + return + try: + loop = asyncio.get_running_loop() + except RuntimeError: + return + task = loop.create_task(self.track_event(event)) + self._pending_event_tasks.add(task) + task.add_done_callback(self._pending_event_tasks.discard) def close(self): """Release all resources held by this Datasette instance. diff --git a/datasette/events.py b/datasette/events.py index 5f3fd06eb6..3583be4406 100644 --- a/datasette/events.py +++ b/datasette/events.py @@ -241,6 +241,54 @@ class DeleteRowEvent(Event): pks: list +@dataclass +class AddDatabaseEvent(Event): + """ + Event name: ``add-database`` + + A new database was attached to this Datasette instance while it was + running, using :ref:`datasette.add_database() `. + + :ivar database: The name the database was attached as. + :type database: str + :ivar path: Resolved absolute filesystem path to the database file, or ``None`` for in-memory databases. + :type path: str or None + :ivar is_memory: ``True`` if this is an in-memory database. + :type is_memory: bool + """ + + name = "add-database" + database: str + path: str | None + is_memory: bool + + +@dataclass +class RemoveDatabaseEvent(Event): + """ + Event name: ``remove-database`` + + A database was detached from this Datasette instance using + :ref:`datasette.remove_database() `. The + database file itself is not deleted by Datasette core, and any queued + writes have been flushed by the time this event is delivered - with one + exception: temporary on-disk databases remove their backing file when + they are closed, so for those the path in this event no longer exists. + + :ivar database: The name the database was attached as. + :type database: str + :ivar path: Resolved absolute filesystem path to the database file, or ``None`` for in-memory databases. + :type path: str or None + :ivar is_memory: ``True`` if this was an in-memory database. + :type is_memory: bool + """ + + name = "remove-database" + database: str + path: str | None + is_memory: bool + + @hookimpl def write_wrapper(datasette, database, request, transaction): def wrapper(conn, track_event): @@ -291,4 +339,6 @@ def register_events(): UpsertRowsEvent, UpdateRowEvent, DeleteRowEvent, + AddDatabaseEvent, + RemoveDatabaseEvent, ] diff --git a/docs/events.md b/docs/events.md index f63d189343..c3c71057c1 100644 --- a/docs/events.md +++ b/docs/events.md @@ -9,6 +9,15 @@ Note that these events will *not* fire for changes made to a SQLite database by Plugins can listen for events using the {ref}`plugin_hook_track_event` plugin hook, which will be called with instances of the following classes - or additional classes {ref}`registered by other plugins `. +## Delivery guarantees for database lifecycle events + +The ``add-database`` and ``remove-database`` events have some specific delivery characteristics: + +- Delivery is asynchronous. Listeners run shortly after the change, not before the triggering ``add_database()`` or ``remove_database()`` call returns. +- These events fire only for changes made at runtime - while an event loop is running, after Datasette's startup has completed. Databases attached while the instance is starting up do not produce events: plugins that need to see those should iterate over ``datasette.databases`` in their own {ref}`plugin_hook_startup` hook. +- Rapid successive changes involving the same database name may reach listeners interleaved. Listeners should tolerate events arriving out of order. +- ``event.actor`` is ``None`` for programmatic calls made by Datasette itself or by plugins. + ```{eval-rst} .. automodule:: datasette.events :members: diff --git a/docs/internals.rst b/docs/internals.rst index d2bd46efb5..34e29e2cbd 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -1357,6 +1357,8 @@ Use ``is_mutable=False`` to add an immutable database. "CREATE TABLE foo(id integer primary key)" ) +Calling this method while the instance is running - after ``invoke_startup()`` has completed, with an event loop running - emits an ``add-database`` :ref:`event `. Databases attached during startup do not emit events: plugins that need to see those should iterate over ``datasette.databases`` in their own :ref:`plugin_hook_startup` hook. + .. _datasette_add_memory_database: .add_memory_database(memory_name, name=None, route=None) @@ -1392,6 +1394,8 @@ The ``name`` and ``route`` parameters are optional and work the same way as they This removes a database that has been previously added. ``name=`` is the unique name of that database. +The database is closed but its file is not deleted. When called while the instance is running this emits a ``remove-database`` :ref:`event ` after the database has been closed - since closing flushes any queued writes first, an event listener can safely perform a final read of the database file. The exception is temporary on-disk databases, which remove their backing file when closed. + .. _datasette_close: .close() diff --git a/tests/test_internals_datasette.py b/tests/test_internals_datasette.py index ed2aeaf0b7..e3d9b8239e 100644 --- a/tests/test_internals_datasette.py +++ b/tests/test_internals_datasette.py @@ -476,3 +476,98 @@ class ExampleContext(Context): await ds.invoke_startup() rendered = await ds.render_template("error.html", context) assert "shallow-copied-value" in rendered + + +def _lifecycle_events(ds): + return [ + event + for event in getattr(ds, "_tracked_events", []) + if event.name in ("add-database", "remove-database") + ] + + +async def _drain_event_tasks(ds): + await asyncio.gather(*ds._pending_event_tasks) + + +@pytest.mark.asyncio +async def test_add_database_fires_event(tmp_path): + ds = Datasette(memory=True) + await ds.invoke_startup() + path = str(tmp_path / "data.db") + sqlite3.connect(path).execute("vacuum") + db = ds.add_database(Database(ds, path=path, is_mutable=True)) + await _drain_event_tasks(ds) + events = _lifecycle_events(ds) + assert len(events) == 1 + event = events[0] + assert event.name == "add-database" + assert event.database == db.name == "data" + assert event.path == os.path.abspath(path) + assert event.is_memory is False + assert event.actor is None + + +@pytest.mark.asyncio +async def test_add_memory_database_fires_event(): + ds = Datasette(memory=True) + await ds.invoke_startup() + ds.add_memory_database("test_add_memory_database_event") + await _drain_event_tasks(ds) + events = _lifecycle_events(ds) + assert len(events) == 1 + event = events[0] + assert event.name == "add-database" + assert event.database == "test_add_memory_database_event" + assert event.path is None + assert event.is_memory is True + + +@pytest.mark.asyncio +async def test_remove_database_fires_event(tmp_path): + ds = Datasette(memory=True) + await ds.invoke_startup() + path = str(tmp_path / "data.db") + sqlite3.connect(path).execute("vacuum") + db = ds.add_database(Database(ds, path=path, is_mutable=True)) + ds.remove_database(db.name) + await _drain_event_tasks(ds) + events = _lifecycle_events(ds) + assert [event.name for event in events] == ["add-database", "remove-database"] + event = events[1] + assert event.database == "data" + assert event.path == os.path.abspath(path) + assert event.is_memory is False + assert event.actor is None + # remove_database never deletes the file + assert os.path.exists(path) + + +@pytest.mark.asyncio +async def test_add_database_no_event_before_startup(): + ds = Datasette(memory=True) + # invoke_startup() has not run - no event, and no AssertionError from + # track_event()'s event_classes check + ds.add_database(Database(ds, memory_name="pre_startup_db")) + assert ds._pending_event_tasks == set() + assert _lifecycle_events(ds) == [] + + +def test_add_database_no_event_without_running_loop(): + ds = Datasette(memory=True) + asyncio.run(ds.invoke_startup()) + # Startup has run but there is no running event loop now + ds.add_database(Database(ds, memory_name="no_loop_db")) + assert ds._pending_event_tasks == set() + assert _lifecycle_events(ds) == [] + + +@pytest.mark.asyncio +async def test_add_database_event_uses_renamed_name(): + ds = Datasette(memory=True) + await ds.invoke_startup() + ds.add_memory_database("first_mem", name="clash") + ds.add_memory_database("second_mem", name="clash") + await _drain_event_tasks(ds) + events = _lifecycle_events(ds) + assert [event.database for event in events] == ["clash", "clash_2"] diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 734f0fc290..1c00bc9cc3 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -1661,6 +1661,9 @@ async def test_hook_register_events(): datasette = Datasette(memory=True) await datasette.invoke_startup() assert any(k.__name__ == "OneEvent" for k in datasette.event_classes) + # Core database lifecycle events should be registered too + registered_names = {k.name for k in datasette.event_classes} + assert {"add-database", "remove-database"} <= registered_names @pytest.mark.asyncio