From 484bef0d3b628c77e7331ddd633d68c4a66817f3 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 29 Nov 2022 10:06:19 -0800 Subject: [PATCH] /db/table/pk/-/update endpoint, closes #1863 --- datasette/app.py | 6 +- datasette/default_permissions.py | 1 + datasette/views/row.py | 100 ++++++++++++++++++----- docs/authentication.rst | 12 +++ docs/json_api.rst | 59 +++++++++++++- tests/test_api_write.py | 134 ++++++++++++++++++++++++++----- 6 files changed, 269 insertions(+), 43 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 9040f059..7aa2ac4b 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -41,7 +41,7 @@ from .views.special import ( MessagesDebugView, ) from .views.table import TableView, TableInsertView, TableDropView -from .views.row import RowView, RowDeleteView +from .views.row import RowView, RowDeleteView, RowUpdateView from .renderer import json_renderer from .url_builder import Urls from .database import Database, QueryInterrupted @@ -1298,6 +1298,10 @@ class Datasette: RowDeleteView.as_view(self), r"/(?P[^\/\.]+)/(?P[^/]+?)/(?P[^/]+?)/-/delete$", ) + add_route( + RowUpdateView.as_view(self), + r"/(?P[^\/\.]+)/(?P
[^/]+?)/(?P[^/]+?)/-/update$", + ) return [ # Compile any strings to regular expressions ((re.compile(pattern) if isinstance(pattern, str) else pattern), view) diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index daaec8a7..2aaf72d6 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -16,6 +16,7 @@ def permission_allowed_default(datasette, actor, action, resource): "create-table", "drop-table", "delete-row", + "update-row", ): if actor and actor.get("id") == "root": return True diff --git a/datasette/views/row.py b/datasette/views/row.py index dc93c3be..b3c45746 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -148,6 +148,27 @@ class RowError(Exception): self.error = error +async def _resolve_row_and_check_permission(datasette, request, permission): + from datasette.app import DatabaseNotFound, TableNotFound, RowNotFound + + try: + resolved = await datasette.resolve_row(request) + except DatabaseNotFound as e: + return False, _error(["Database not found: {}".format(e.database_name)], 404) + except TableNotFound as e: + return False, _error(["Table not found: {}".format(e.table)], 404) + except RowNotFound as e: + return False, _error(["Record not found: {}".format(e.pk_values)], 404) + + # Ensure user has permission to delete this row + if not await datasette.permission_allowed( + request.actor, permission, resource=(resolved.db.name, resolved.table) + ): + return False, _error(["Permission denied"], 403) + + return True, resolved + + class RowDeleteView(BaseView): name = "row-delete" @@ -155,30 +176,65 @@ class RowDeleteView(BaseView): self.ds = datasette async def post(self, request): - from datasette.app import DatabaseNotFound, TableNotFound, RowNotFound - - try: - resolved = await self.ds.resolve_row(request) - except DatabaseNotFound as e: - return _error(["Database not found: {}".format(e.database_name)], 404) - except TableNotFound as e: - return _error(["Table not found: {}".format(e.table)], 404) - except RowNotFound as e: - return _error(["Record not found: {}".format(e.pk_values)], 404) - db = resolved.db - database_name = db.name - table = resolved.table - pk_values = resolved.pk_values - - # Ensure user has permission to delete this row - if not await self.ds.permission_allowed( - request.actor, "delete-row", resource=(database_name, table) - ): - return _error(["Permission denied"], 403) + ok, resolved = await _resolve_row_and_check_permission( + self.ds, request, "delete-row" + ) + if not ok: + return resolved # Delete table def delete_row(conn): - sqlite_utils.Database(conn)[table].delete(pk_values) + sqlite_utils.Database(conn)[resolved.table].delete(resolved.pk_values) + + try: + await resolved.db.execute_write_fn(delete_row) + except Exception as e: + return _error([str(e)], 500) - await db.execute_write_fn(delete_row) return Response.json({"ok": True}, status=200) + + +class RowUpdateView(BaseView): + name = "row-update" + + def __init__(self, datasette): + self.ds = datasette + + async def post(self, request): + ok, resolved = await _resolve_row_and_check_permission( + self.ds, request, "update-row" + ) + if not ok: + return resolved + + body = await request.post_body() + try: + data = json.loads(body) + except json.JSONDecodeError as e: + return _error(["Invalid JSON: {}".format(e)]) + + if not isinstance(data, dict): + return _error(["JSON must be a dictionary"]) + if not "update" in data or not isinstance(data["update"], dict): + return _error(["JSON must contain an update dictionary"]) + + update = data["update"] + + def update_row(conn): + sqlite_utils.Database(conn)[resolved.table].update( + resolved.pk_values, update + ) + + try: + await resolved.db.execute_write_fn(update_row) + except Exception as e: + return _error([str(e)], 400) + + result = {"ok": True} + if data.get("return"): + results = await resolved.db.execute( + resolved.sql, resolved.params, truncate=True + ) + rows = list(results.rows) + result["row"] = dict(rows[0]) + return Response.json(result, status=200) diff --git a/docs/authentication.rst b/docs/authentication.rst index a86c82a2..5881143a 100644 --- a/docs/authentication.rst +++ b/docs/authentication.rst @@ -589,6 +589,18 @@ Actor is allowed to delete rows from a table. Default *deny*. +.. _permissions_update_row: + +update-row +---------- + +Actor is allowed to update rows in a table. + +``resource`` - tuple: (string, string) + The name of the database, then the name of the table + +Default *deny*. + .. _permissions_create_table: create-table diff --git a/docs/json_api.rst b/docs/json_api.rst index ffdbf8d3..06a1d65d 100644 --- a/docs/json_api.rst +++ b/docs/json_api.rst @@ -548,10 +548,65 @@ To return the newly inserted rows, add the ``"return": true`` key to the request This will return the same ``"rows"`` key as the single row example above. There is a small performance penalty for using this option. +.. _RowUpdateView: + +Updating a row +~~~~~~~~~~~~~~ + +To update a row, make a ``POST`` to ``//
//-/update``. This requires the :ref:`permissions_update_row` permission. + +:: + + POST //
//-/update + Content-Type: application/json + Authorization: Bearer dstok_ + +.. code-block:: json + + { + "update": { + "text_column": "New text string", + "integer_column": 3, + "float_column": 3.14 + } + } + +```` here is the :ref:`tilde-encoded ` primary key value of the row to delete - or a comma-separated list of primary key values if the table has a composite primary key. + +You only need to pass the columns you want to update. Any other columns will be left unchanged. + +If successful, this will return a ``200`` status code and a ``{"ok": true}`` response body. + +Add ``"return": true`` to the request body to return the updated row: + +.. code-block:: json + + { + "update": { + "title": "New title" + }, + "return": true + } + +The returned JSON will look like this: + +.. code-block:: json + + { + "ok": true, + "row": { + "id": 1, + "title": "New title", + "other_column": "Will be present here too" + } + } + +Any errors will return ``{"errors": ["... descriptive message ..."], "ok": false}``, and a ``400`` status code for a bad input or a ``403`` status code for an authentication or permission error. + .. _RowDeleteView: -Deleting rows -~~~~~~~~~~~~~ +Deleting a row +~~~~~~~~~~~~~~ To delete a row, make a ``POST`` to ``//
//-/delete``. This requires the :ref:`permissions_delete_row` permission. diff --git a/tests/test_api_write.py b/tests/test_api_write.py index f455775b..0479a2dd 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -14,7 +14,7 @@ def ds_write(tmp_path_factory): for db in (db1, db2): db.execute("vacuum") db.execute( - "create table docs (id integer primary key, title text, score float)" + "create table docs (id integer primary key, title text, score float, age integer)" ) ds = Datasette([db_path], immutables=[db_path_immutable]) yield ds @@ -34,13 +34,13 @@ async def test_write_row(ds_write): token = write_token(ds_write) response = await ds_write.client.post( "/data/docs/-/insert", - json={"row": {"title": "Test", "score": 1.0}}, + json={"row": {"title": "Test", "score": 1.2, "age": 5}}, headers={ "Authorization": "Bearer {}".format(token), "Content-Type": "application/json", }, ) - expected_row = {"id": 1, "title": "Test", "score": 1.0} + expected_row = {"id": 1, "title": "Test", "score": 1.2, "age": 5} assert response.status_code == 201 assert response.json()["rows"] == [expected_row] rows = (await ds_write.get_database("data").execute("select * from docs")).rows @@ -51,7 +51,11 @@ async def test_write_row(ds_write): @pytest.mark.parametrize("return_rows", (True, False)) async def test_write_rows(ds_write, return_rows): token = write_token(ds_write) - data = {"rows": [{"title": "Test {}".format(i), "score": 1.0} for i in range(20)]} + data = { + "rows": [ + {"title": "Test {}".format(i), "score": 1.0, "age": 5} for i in range(20) + ] + } if return_rows: data["return"] = True response = await ds_write.client.post( @@ -71,7 +75,8 @@ async def test_write_rows(ds_write, return_rows): ] assert len(actual_rows) == 20 assert actual_rows == [ - {"id": i + 1, "title": "Test {}".format(i), "score": 1.0} for i in range(20) + {"id": i + 1, "title": "Test {}".format(i), "score": 1.0, "age": 5} + for i in range(20) ] assert response.json()["ok"] is True if return_rows: @@ -241,14 +246,14 @@ async def test_write_row_errors( True, False, [ - {"id": 1, "title": "Exists", "score": None}, + {"id": 1, "title": "Exists", "score": None, "age": None}, ], ), ( False, True, [ - {"id": 1, "title": "One", "score": None}, + {"id": 1, "title": "One", "score": None, "age": None}, ], ), ), @@ -289,6 +294,19 @@ async def test_insert_ignore_replace( assert response.json()["rows"] == expected_rows +async def _insert_row(ds): + insert_response = await ds.client.post( + "/data/docs/-/insert", + json={"row": {"title": "Row one", "score": 1.2, "age": 5}, "return": True}, + headers={ + "Authorization": "Bearer {}".format(write_token(ds)), + "Content-Type": "application/json", + }, + ) + assert insert_response.status_code == 201 + return insert_response.json()["rows"][0]["id"] + + @pytest.mark.asyncio @pytest.mark.parametrize("scenario", ("no_token", "no_perm", "bad_table", "has_perm")) async def test_delete_row(ds_write, scenario): @@ -300,17 +318,7 @@ async def test_delete_row(ds_write, scenario): token = write_token(ds_write) should_work = scenario == "has_perm" - # Insert a row - insert_response = await ds_write.client.post( - "/data/docs/-/insert", - json={"row": {"title": "Row one", "score": 1.0}, "return": True}, - headers={ - "Authorization": "Bearer {}".format(write_token(ds_write)), - "Content-Type": "application/json", - }, - ) - assert insert_response.status_code == 201 - pk = insert_response.json()["rows"][0]["id"] + pk = await _insert_row(ds_write) path = "/data/{}/{}/-/delete".format( "docs" if scenario != "bad_table" else "bad_table", pk @@ -343,6 +351,96 @@ async def test_delete_row(ds_write, scenario): ) +@pytest.mark.asyncio +@pytest.mark.parametrize("scenario", ("no_token", "no_perm", "bad_table")) +async def test_update_row_check_permission(ds_write, scenario): + if scenario == "no_token": + token = "bad_token" + elif scenario == "no_perm": + token = write_token(ds_write, actor_id="not-root") + else: + token = write_token(ds_write) + + pk = await _insert_row(ds_write) + + path = "/data/{}/{}/-/delete".format( + "docs" if scenario != "bad_table" else "bad_table", pk + ) + + response = await ds_write.client.post( + path, + json={"update": {"title": "New title"}}, + headers={ + "Authorization": "Bearer {}".format(token), + "Content-Type": "application/json", + }, + ) + assert response.status_code == 403 if scenario in ("no_token", "bad_token") else 404 + assert response.json()["ok"] is False + assert ( + response.json()["errors"] == ["Permission denied"] + if scenario == "no_token" + else ["Table not found: bad_table"] + ) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "input,expected_errors", + ( + ({"title": "New title"}, None), + ({"title": None}, None), + ({"score": 1.6}, None), + ({"age": 10}, None), + ({"title": "New title", "score": 1.6}, None), + ({"title2": "New title"}, ["no such column: title2"]), + ), +) +@pytest.mark.parametrize("use_return", (True, False)) +async def test_update_row(ds_write, input, expected_errors, use_return): + token = write_token(ds_write) + pk = await _insert_row(ds_write) + + path = "/data/docs/{}/-/update".format(pk) + + data = {"update": input} + if use_return: + data["return"] = True + + response = await ds_write.client.post( + path, + json=data, + headers={ + "Authorization": "Bearer {}".format(token), + "Content-Type": "application/json", + }, + ) + if expected_errors: + assert response.status_code == 400 + assert response.json()["ok"] is False + assert response.json()["errors"] == expected_errors + return + + assert response.json()["ok"] is True + if not use_return: + assert "row" not in response.json() + else: + returned_row = response.json()["row"] + assert returned_row["id"] == pk + for k, v in input.items(): + assert returned_row[k] == v + + # And fetch the row to check it's updated + response = await ds_write.client.get( + "/data/docs/{}.json?_shape=array".format(pk), + ) + assert response.status_code == 200 + row = response.json()[0] + assert row["id"] == pk + for k, v in input.items(): + assert row[k] == v + + @pytest.mark.asyncio @pytest.mark.parametrize( "scenario", ("no_token", "no_perm", "bad_table", "has_perm", "immutable")