Skip to content

Commit

Permalink
alter: true support for /-/insert and /-/upsert, refs #2101
Browse files Browse the repository at this point in the history
  • Loading branch information
simonw committed Feb 8, 2024
1 parent b5ccc4d commit 528d89d
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 11 deletions.
48 changes: 42 additions & 6 deletions datasette/views/table.py
Expand Up @@ -8,7 +8,12 @@

from datasette.plugins import pm
from datasette.database import QueryInterrupted
from datasette.events import DropTableEvent, InsertRowsEvent, UpsertRowsEvent
from datasette.events import (
AlterTableEvent,
DropTableEvent,
InsertRowsEvent,
UpsertRowsEvent,
)
from datasette import tracer
from datasette.utils import (
add_cors_headers,
Expand Down Expand Up @@ -388,7 +393,7 @@ def _errors(errors):
extras = {
key: value for key, value in data.items() if key not in ("row", "rows")
}
valid_extras = {"return", "ignore", "replace"}
valid_extras = {"return", "ignore", "replace", "alter"}
invalid_extras = extras.keys() - valid_extras
if invalid_extras:
return _errors(
Expand All @@ -397,7 +402,6 @@ def _errors(errors):
if extras.get("ignore") and extras.get("replace"):
return _errors(['Cannot use "ignore" and "replace" at the same time'])

# Validate columns of each row
columns = set(await db.table_columns(table_name))
columns.update(pks_list)

Expand All @@ -412,7 +416,7 @@ def _errors(errors):
)
)
invalid_columns = set(row.keys()) - columns
if invalid_columns:
if invalid_columns and not extras.get("alter"):
errors.append(
"Row {} has invalid columns: {}".format(
i, ", ".join(sorted(invalid_columns))
Expand Down Expand Up @@ -476,10 +480,23 @@ async def post(self, request, upsert=False):

ignore = extras.get("ignore")
replace = extras.get("replace")
alter = extras.get("alter")

if upsert and (ignore or replace):
return _error(["Upsert does not support ignore or replace"], 400)

initial_schema = None
if alter:
# Must have alter-table permission
if not await self.ds.permission_allowed(
request.actor, "alter-table", resource=(database_name, table_name)
):
return _error(["Permission denied for alter-table"], 403)
# Track initial schema to check if it changed later
initial_schema = await db.execute_fn(
lambda conn: sqlite_utils.Database(conn)[table_name].schema
)

should_return = bool(extras.get("return", False))
row_pk_values_for_later = []
if should_return and upsert:
Expand All @@ -489,9 +506,13 @@ def insert_or_upsert_rows(conn):
table = sqlite_utils.Database(conn)[table_name]
kwargs = {}
if upsert:
kwargs["pk"] = pks[0] if len(pks) == 1 else pks
kwargs = {
"pk": pks[0] if len(pks) == 1 else pks,
"alter": alter,
}
else:
kwargs = {"ignore": ignore, "replace": replace}
# Insert
kwargs = {"ignore": ignore, "replace": replace, "alter": alter}
if should_return and not upsert:
rowids = []
method = table.upsert if upsert else table.insert
Expand Down Expand Up @@ -552,6 +573,21 @@ def insert_or_upsert_rows(conn):
)
)

if initial_schema is not None:
after_schema = await db.execute_fn(
lambda conn: sqlite_utils.Database(conn)[table_name].schema
)
if initial_schema != after_schema:
await self.ds.track_event(
AlterTableEvent(
request.actor,
database=database_name,
table=table_name,
before_schema=initial_schema,
after_schema=after_schema,
)
)

return Response.json(result, status=200 if upsert else 201)


Expand Down
6 changes: 5 additions & 1 deletion docs/json_api.rst
Expand Up @@ -618,6 +618,8 @@ Pass ``"ignore": true`` to ignore these errors and insert the other rows:
Or you can pass ``"replace": true`` to replace any rows with conflicting primary keys with the new values.

Pass ``"alter: true`` to automatically add any missing columns to the table. This requires the :ref:`permissions_alter_table` permission.

.. _TableUpsertView:

Upserting rows
Expand Down Expand Up @@ -728,6 +730,8 @@ When using upsert you must provide the primary key column (or columns if the tab
If your table does not have an explicit primary key you should pass the SQLite ``rowid`` key instead.

Pass ``"alter: true`` to automatically add any missing columns to the table. This requires the :ref:`permissions_alter_table` permission.

.. _RowUpdateView:

Updating a row
Expand Down Expand Up @@ -849,7 +853,7 @@ The JSON here describes the table that will be created:
* ``pks`` can be used instead of ``pk`` to create a compound primary key. It should be a JSON list of column names to use in that primary key.
* ``ignore`` can be set to ``true`` to ignore existing rows by primary key if the table already exists.
* ``replace`` can be set to ``true`` to replace existing rows by primary key if the table already exists.
* ``alter`` can be set to ``true`` if you want to automatically add any missing columns to the table. This requires the :ref:`permissions_alter_table` permission.
* ``alter`` can be set to ``true`` if you want to automatically add any missing columns to the table. This requires the :ref:`permissions_alter_table` permission.

If the table is successfully created this will return a ``201`` status code and the following response:

Expand Down
48 changes: 44 additions & 4 deletions tests/test_api_write.py
Expand Up @@ -60,6 +60,27 @@ async def test_insert_row(ds_write):
assert not event.replace


@pytest.mark.asyncio
async def test_insert_row_alter(ds_write):
token = write_token(ds_write)
response = await ds_write.client.post(
"/data/docs/-/insert",
json={
"row": {"title": "Test", "score": 1.2, "age": 5, "extra": "extra"},
"alter": True,
},
headers=_headers(token),
)
assert response.status_code == 201
assert response.json()["ok"] is True
assert response.json()["rows"][0]["extra"] == "extra"
# Analytics event
event = last_event(ds_write)
assert event.name == "alter-table"
assert "extra" not in event.before_schema
assert "extra" in event.after_schema


@pytest.mark.asyncio
@pytest.mark.parametrize("return_rows", (True, False))
async def test_insert_rows(ds_write, return_rows):
Expand Down Expand Up @@ -278,16 +299,27 @@ async def test_insert_rows(ds_write, return_rows):
403,
["Permission denied: need both insert-row and update-row"],
),
# Alter table forbidden without alter permission
(
"/data/docs/-/upsert",
{"rows": [{"id": 1, "title": "One", "extra": "extra"}], "alter": True},
"update-and-insert-but-no-alter",
403,
["Permission denied for alter-table"],
),
),
)
async def test_insert_or_upsert_row_errors(
ds_write, path, input, special_case, expected_status, expected_errors
):
token = write_token(ds_write)
token_permissions = []
if special_case == "insert-but-not-update":
token = write_token(ds_write, permissions=["ir", "vi"])
token_permissions = ["ir", "vi"]
if special_case == "update-but-not-insert":
token = write_token(ds_write, permissions=["ur", "vi"])
token_permissions = ["ur", "vi"]
if special_case == "update-and-insert-but-no-alter":
token_permissions = ["ur", "ir"]
token = write_token(ds_write, permissions=token_permissions)
if special_case == "duplicate_id":
await ds_write.get_database("data").execute_write(
"insert into docs (id) values (1)"
Expand All @@ -309,7 +341,9 @@ async def test_insert_or_upsert_row_errors(
actor_response = (
await ds_write.client.get("/-/actor.json", headers=kwargs["headers"])
).json()
print(actor_response)
assert set((actor_response["actor"] or {}).get("_r", {}).get("a") or []) == set(
token_permissions
)

if special_case == "invalid_json":
del kwargs["json"]
Expand Down Expand Up @@ -434,6 +468,12 @@ async def test_insert_ignore_replace(
{"id": 1, "title": "Two", "score": 1},
],
),
(
# Upsert with an alter
{"rows": [{"id": 1, "title": "One"}], "pk": "id"},
{"rows": [{"id": 1, "title": "Two", "extra": "extra"}], "alter": True},
[{"id": 1, "title": "Two", "extra": "extra"}],
),
),
)
@pytest.mark.parametrize("should_return", (False, True))
Expand Down

0 comments on commit 528d89d

Please sign in to comment.