From 3a999a85fb431594ccee1adf38721de03de19500 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 16 Feb 2024 13:58:33 -0800 Subject: [PATCH] Fire insert-rows on /db/-/create if rows were inserted, refs #2260 --- datasette/views/database.py | 13 ++++++- tests/test_api_write.py | 71 +++++++++++++++++++++++++++++-------- 2 files changed, 69 insertions(+), 15 deletions(-) diff --git a/datasette/views/database.py b/datasette/views/database.py index bd55064f..2a8b40cc 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -10,7 +10,7 @@ import re import sqlite_utils import textwrap -from datasette.events import AlterTableEvent, CreateTableEvent +from datasette.events import AlterTableEvent, CreateTableEvent, InsertRowsEvent from datasette.database import QueryInterrupted from datasette.utils import ( add_cors_headers, @@ -1022,6 +1022,17 @@ class TableCreateView(BaseView): request.actor, database=db.name, table=table_name, schema=schema ) ) + if rows: + await self.ds.track_event( + InsertRowsEvent( + request.actor, + database=db.name, + table=table_name, + num_rows=len(rows), + ignore=ignore, + replace=replace, + ) + ) return Response.json(details, status=201) diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 2aea699b..0eb915ba 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -857,13 +857,14 @@ async def test_drop_table(ds_write, scenario): @pytest.mark.asyncio @pytest.mark.parametrize( - "input,expected_status,expected_response", + "input,expected_status,expected_response,expected_events", ( # Permission error with a bad token ( {"table": "bad", "row": {"id": 1}}, 403, {"ok": False, "errors": ["Permission denied"]}, + [], ), # Successful creation with columns: ( @@ -910,6 +911,7 @@ async def test_drop_table(ds_write, scenario): ")" ), }, + ["create-table"], ), # Successful creation with rows: ( @@ -945,6 +947,7 @@ async def test_drop_table(ds_write, scenario): ), "row_count": 2, }, + ["create-table", "insert-rows"], ), # Successful creation with row: ( @@ -973,6 +976,7 @@ async def test_drop_table(ds_write, scenario): ), "row_count": 1, }, + ["create-table", "insert-rows"], ), # Create with row and no primary key ( @@ -992,6 +996,7 @@ async def test_drop_table(ds_write, scenario): "schema": ("CREATE TABLE [four] (\n" " [name] TEXT\n" ")"), "row_count": 1, }, + ["create-table", "insert-rows"], ), # Create table with compound primary key ( @@ -1013,6 +1018,7 @@ async def test_drop_table(ds_write, scenario): ), "row_count": 1, }, + ["create-table", "insert-rows"], ), # Error: Table is required ( @@ -1024,6 +1030,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["Table is required"], }, + [], ), # Error: Invalid table name ( @@ -1036,6 +1043,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["Invalid table name"], }, + [], ), # Error: JSON must be an object ( @@ -1045,6 +1053,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["JSON must be an object"], }, + [], ), # Error: Cannot specify columns with rows or row ( @@ -1058,6 +1067,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["Cannot specify columns with rows or row"], }, + [], ), # Error: columns, rows or row is required ( @@ -1069,6 +1079,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["columns, rows or row is required"], }, + [], ), # Error: columns must be a list ( @@ -1081,6 +1092,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["columns must be a list"], }, + [], ), # Error: columns must be a list of objects ( @@ -1093,6 +1105,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["columns must be a list of objects"], }, + [], ), # Error: Column name is required ( @@ -1105,6 +1118,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["Column name is required"], }, + [], ), # Error: Unsupported column type ( @@ -1117,6 +1131,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["Unsupported column type: bad"], }, + [], ), # Error: Duplicate column name ( @@ -1132,6 +1147,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["Duplicate column name: id"], }, + [], ), # Error: rows must be a list ( @@ -1144,6 +1160,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["rows must be a list"], }, + [], ), # Error: rows must be a list of objects ( @@ -1156,6 +1173,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["rows must be a list of objects"], }, + [], ), # Error: pk must be a string ( @@ -1169,6 +1187,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["pk must be a string"], }, + [], ), # Error: Cannot specify both pk and pks ( @@ -1183,6 +1202,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["Cannot specify both pk and pks"], }, + [], ), # Error: pks must be a list ( @@ -1196,12 +1216,14 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["pks must be a list"], }, + [], ), # Error: pks must be a list of strings ( {"table": "bad", "row": {"id": 1, "name": "Row 1"}, "pks": [1, 2]}, 400, {"ok": False, "errors": ["pks must be a list of strings"]}, + [], ), # Error: ignore and replace are mutually exclusive ( @@ -1217,6 +1239,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["ignore and replace are mutually exclusive"], }, + [], ), # ignore and replace require row or rows ( @@ -1230,6 +1253,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["ignore and replace require row or rows"], }, + [], ), # ignore and replace require pk or pks ( @@ -1243,6 +1267,7 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["ignore and replace require pk or pks"], }, + [], ), ( { @@ -1255,10 +1280,14 @@ async def test_drop_table(ds_write, scenario): "ok": False, "errors": ["ignore and replace require pk or pks"], }, + [], ), ), ) -async def test_create_table(ds_write, input, expected_status, expected_response): +async def test_create_table( + ds_write, input, expected_status, expected_response, expected_events +): + ds_write._tracked_events = [] # Special case for expected status of 403 if expected_status == 403: token = "bad_token" @@ -1272,12 +1301,9 @@ async def test_create_table(ds_write, input, expected_status, expected_response) assert response.status_code == expected_status data = response.json() assert data == expected_response - # create-table event - if expected_status == 201: - event = last_event(ds_write) - assert event.name == "create-table" - assert event.actor == {"id": "root", "token": "dstok"} - assert event.schema.startswith("CREATE TABLE ") + # Should have tracked the expected events + events = ds_write._tracked_events + assert [e.name for e in events] == expected_events @pytest.mark.asyncio @@ -1376,6 +1402,8 @@ async def test_create_table_ignore_replace(ds_write, input, expected_rows_after) ) assert first_response.status_code == 201 + ds_write._tracked_events = [] + # Try a second time second_response = await ds_write.client.post( "/data/-/create", @@ -1387,6 +1415,10 @@ async def test_create_table_ignore_replace(ds_write, input, expected_rows_after) rows = await ds_write.client.get("/data/test_insert_replace.json?_shape=array") assert rows.json() == expected_rows_after + # Check it fired the right events + event_names = [e.name for e in ds_write._tracked_events] + assert event_names == ["insert-rows"] + @pytest.mark.asyncio async def test_create_table_error_if_pk_changed(ds_write): @@ -1471,6 +1503,7 @@ async def test_method_not_allowed(ds_write, path): @pytest.mark.asyncio async def test_create_uses_alter_by_default_for_new_table(ds_write): + ds_write._tracked_events = [] token = write_token(ds_write) response = await ds_write.client.post( "/data/-/create", @@ -1490,8 +1523,8 @@ async def test_create_uses_alter_by_default_for_new_table(ds_write): headers=_headers(token), ) assert response.status_code == 201 - event = last_event(ds_write) - assert event.name == "create-table" + event_names = [e.name for e in ds_write._tracked_events] + assert event_names == ["create-table", "insert-rows"] @pytest.mark.asyncio @@ -1517,6 +1550,8 @@ async def test_create_using_alter_against_existing_table( headers=_headers(token), ) assert response.status_code == 201 + + ds_write._tracked_events = [] # Now try to insert more rows using /-/create with alter=True response2 = await ds_write.client.post( "/data/-/create", @@ -1536,8 +1571,16 @@ async def test_create_using_alter_against_existing_table( } else: assert response2.status_code == 201 + + event_names = [e.name for e in ds_write._tracked_events] + assert event_names == ["alter-table", "insert-rows"] + # It should have altered the table - event = last_event(ds_write) - assert event.name == "alter-table" - assert "extra" not in event.before_schema - assert "extra" in event.after_schema + alter_event = ds_write._tracked_events[0] + assert alter_event.name == "alter-table" + assert "extra" not in alter_event.before_schema + assert "extra" in alter_event.after_schema + + insert_rows_event = ds_write._tracked_events[1] + assert insert_rows_event.name == "insert-rows" + assert insert_rows_event.num_rows == 1