diff --git a/datasette/views/index.py b/datasette/views/index.py index 6a9462ac..86f8ae7d 100644 --- a/datasette/views/index.py +++ b/datasette/views/index.py @@ -151,6 +151,7 @@ class IndexView(BaseView): return Response( json.dumps( { + "ok": True, "databases": {db["name"]: db for db in databases}, "metadata": await self.ds.get_instance_metadata(), }, diff --git a/datasette/views/special.py b/datasette/views/special.py index 602ec5ea..c289a240 100644 --- a/datasette/views/special.py +++ b/datasette/views/special.py @@ -63,6 +63,8 @@ class JsonDataView(BaseView): headers = {} if self.ds.cors: add_cors_headers(headers) + if isinstance(data, dict): + data = {"ok": True, **data} return Response.json(data, headers=headers) else: context = { @@ -414,6 +416,7 @@ class AllowedResourcesView(BaseView): # If catalog tables don't exist yet, return empty results return ( { + "ok": True, "action": action, "actor_id": actor_id, "page": page, @@ -448,6 +451,7 @@ class AllowedResourcesView(BaseView): return f"{request.path}?{query}" response = { + "ok": True, "action": action, "actor_id": actor_id, "page": page, @@ -573,6 +577,7 @@ class PermissionRulesView(BaseView): return f"{request.path}?{query}" response = { + "ok": True, "action": action, "actor_id": (actor or {}).get("id") if actor else None, "page": page, @@ -623,6 +628,7 @@ async def _check_permission_for_actor(ds, action, parent, child, actor): allowed = await ds.allowed(action=action, resource=resource_obj, actor=actor) response = { + "ok": True, "action": action, "allowed": bool(allowed), "resource": { @@ -1208,7 +1214,7 @@ class JumpView(BaseView): match["display_name"] = row["display_name"] matches.append(match) - return Response.json({"matches": matches, "truncated": truncated}) + return Response.json({"ok": True, "matches": matches, "truncated": truncated}) class SchemaBaseView(BaseView): @@ -1230,7 +1236,7 @@ class SchemaBaseView(BaseView): headers = {} if self.ds.cors: add_cors_headers(headers) - return Response.json(data, headers=headers) + return Response.json({"ok": True, **data}, headers=headers) def format_error_response(self, error_message, format_, status=404): """Format error response based on requested format.""" diff --git a/datasette/views/table.py b/datasette/views/table.py index 1fc151e6..ef9831b2 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -1525,7 +1525,7 @@ class TableAutocompleteView(BaseView): and value_as_boolean(initial_arg) ) if not q and not initial: - return Response.json({"rows": []}) + return Response.json({"ok": True, "rows": []}) params = { "q": q, "like": "%{}%".format(_escape_like(q)), @@ -1588,10 +1588,13 @@ class TableAutocompleteView(BaseView): custom_time_limit=AUTOCOMPLETE_TIME_LIMIT_MS, ) except QueryInterrupted: - return Response.json({"rows": []}) + return Response.json({"ok": True, "rows": []}) return Response.json( - {"rows": _autocomplete_response_rows(results.rows, pks, label_column)} + { + "ok": True, + "rows": _autocomplete_response_rows(results.rows, pks, label_column), + } ) diff --git a/docs/introspection.rst b/docs/introspection.rst index 4834f441..d0780763 100644 --- a/docs/introspection.rst +++ b/docs/introspection.rst @@ -7,6 +7,8 @@ Datasette includes some pages and JSON API endpoints for introspecting the curre Each of these pages can be viewed in your browser. Add ``.json`` to the URL to get back the contents as JSON. +JSON responses that return an object include an ``"ok": true`` key, consistent with the rest of the :ref:`JSON API `. + .. _JsonDataView_metadata: /-/metadata @@ -37,6 +39,7 @@ Shows the version of Datasette, Python and SQLite. `Versions example ` for this instance of Datasette. T .. code-block:: json { + "ok": true, "settings": { "template_debug": true, "trace_debug": true, @@ -160,6 +165,7 @@ The endpoint supports a ``?q=`` query parameter for filtering items by name. .. code-block:: json { + "ok": true, "matches": [ { "name": "fixtures", @@ -188,6 +194,7 @@ Search example with ``?q=facet`` returns only items matching ``.*facet.*``: .. code-block:: json { + "ok": true, "matches": [ { "name": "fixtures: facetable", @@ -220,6 +227,7 @@ Shows details of threads and ``asyncio`` tasks. `Threads example jsono?))?$` and `/-/(\.(?Pjsono?))?$` further filtered by `view-database` / `view-table` for the actor. - **Parameters:** `_sort=relationships` sorts each database's truncated table list by foreign-key relationship count. -- **JSON response** (index.py:147-161): +- **JSON response** (index.py:147-161) — includes `ok: true` plus: - `databases` — an **object keyed by database name** (not a list). Each value: `name`, `hash` (or null), `color`, `path`, `tables_and_views_truncated` (up to 5 items: `name`, `columns`, @@ -260,7 +269,7 @@ per-database `view-database` permissions**. app.py:2574-2579, registered with `permission=None` — **accessible to any request including anonymous**. No parameters. -Response: `{"actor": {...}}` or `{"actor": null}` (app.py:2287-2288). +Response: `{"ok": true, "actor": {...}}` or `{"ok": true, "actor": null}` (app.py:2287-2288). ### GET /-/actions(.json) @@ -313,7 +322,7 @@ an optional `.json` suffix but the view **always returns JSON**. `jump_items_sql` plugin hook). - **Parameter:** `q` — whitespace-split terms matched as a case-insensitive `%term1%term2%` LIKE pattern. -- **Response:** `{"matches": [...], "truncated": bool}`; each match: +- **Response:** `{"ok": true, "matches": [...], "truncated": bool}`; each match: `name`, `url`, `type` (`database`/`table`/`view`/`query`/plugin-defined), `description`, optional `display_name`. Capped at 100 matches. @@ -324,7 +333,7 @@ an optional `.json` suffix but the view **always returns JSON**. - **Permission:** no explicit check; only databases the actor can `view-database` are included (others silently omitted). - **Formats:** no extension → HTML; `.json` → - `{"schemas": [{"database": name, "schema": "..."}]}`; `.md` → + `{"ok": true, "schemas": [{"database": name, "schema": "..."}]}`; `.md` → `text/markdown` rendering. ### GET/POST /-/logout @@ -610,10 +619,9 @@ views/table_create_alter.py:965-1005). - **Unknown database** → 404; for `.json`: `{"ok": false, "error": "Database not found"}`. (The existence check runs before the permission check.) -- **Responses:** `.json` → 200 `{"database": "", "schema": ""}` +- **Responses:** `.json` → 200 `{"ok": true, "database": "", "schema": ""}` (concatenated `sqlite_master.sql` joined with `;\n`); `.md` → - `text/markdown`; no extension → HTML. Note the JSON has **no `ok` key** on - success. + `text/markdown`; no extension → HTML. --- @@ -787,8 +795,8 @@ download attachment. In JSON output, binary cells appear as `TableSchemaView` (app.py:2751-2754; views/special.py:1332-1378). - **Permission:** `view-table` via `ensure_permission` (denied → 403 HTML). -- **Responses:** `.json` → 200 `{"database", "table", "schema"}` (no `ok` - key); `.md` → `text/markdown`; no extension → HTML. Missing table → 404 +- **Responses:** `.json` → 200 `{"ok": true, "database", "table", "schema"}`; + `.md` → `text/markdown`; no extension → HTML. Missing table → 404 `{"ok": false, "error": "Table not found"}` for `.json`. ### GET /\/\/-/fragment @@ -805,10 +813,10 @@ only — views get 400 `"Autocomplete is only available for tables"`. - **Permission:** `view-table` (denied → `Forbidden` → 403). - **Parameters:** `q` (matched with escaped `LIKE %q%` against pk columns and the label column) and `_initial` (truthy: with empty `q`, return the 10 - most recent rows). Neither → `{"rows": []}`. + most recent rows). Neither → `{"ok": true, "rows": []}`. - **Response:** `{"rows": [{"pks": {pk_name: value}, "label": "..."}]}` — max 10 items; 500 ms query budget with fallbacks, timing out to - `{"rows": []}`. + `{"ok": true, "rows": []}`. --- diff --git a/stable-api-recommendations.md b/stable-api-recommendations.md index 8d8a87b4..48a864af 100644 --- a/stable-api-recommendations.md +++ b/stable-api-recommendations.md @@ -102,7 +102,15 @@ key — see §2.) --- -## 2. Success envelope: `ok` is not universal, arrays are not extensible (P1/P2) +## 2. Success envelope: `ok` is not universal, arrays are not extensible (P1/P2) — ✅ PARTIALLY IMPLEMENTED + +> **Status:** recommendation 2 and 3 are implemented — every JSON-object +> success response now includes `"ok": true` (`JsonDataView` injects it for +> dict responses; homepage, jump, schema, permission-debug and autocomplete +> views set it explicitly; covered by `tests/test_success_envelope.py`). +> Recommendation 1 (wrapping the `/-/plugins`, `/-/databases`, `/-/actions` +> top-level arrays in objects) is being landed as separate per-endpoint +> commits. §2a/2b/2c remain open. Endpoints disagree about the success envelope: diff --git a/tests/test_api.py b/tests/test_api.py index e5ed1d23..4635b236 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -250,6 +250,7 @@ def test_no_files_uses_memory_database(app_client_no_files): response = app_client_no_files.get("/.json") assert response.status == 200 assert { + "ok": True, "databases": { "_memory": { "name": "_memory", @@ -525,7 +526,7 @@ def test_databases_json(app_client_two_attached_databases_one_immutable): @pytest.mark.asyncio async def test_threads_json(ds_client): response = await ds_client.get("/-/threads.json") - expected_keys = {"threads", "num_threads"} + expected_keys = {"ok", "threads", "num_threads"} if sys.version_info >= (3, 7, 0): expected_keys.update({"tasks", "num_tasks"}) data = response.json() @@ -610,6 +611,7 @@ async def test_actions_json(ds_client): async def test_settings_json(ds_client): response = await ds_client.get("/-/settings.json") assert response.json() == { + "ok": True, "default_page_size": 50, "default_facet_size": 30, "default_allow_sql": True, @@ -884,7 +886,7 @@ async def test_config_json(config, expected): "/-/config.json should return redacted configuration" ds = Datasette(config=config) response = await ds.client.get("/-/config.json") - assert response.json() == expected + assert response.json() == {"ok": True, **expected} @pytest.mark.asyncio @@ -980,7 +982,7 @@ async def test_config_json(config, expected): async def test_upgrade_metadata(metadata, expected_config, expected_metadata): ds = Datasette(metadata=metadata) response = await ds.client.get("/-/config.json") - assert response.json() == expected_config + assert response.json() == {"ok": True, **expected_config} response2 = await ds.client.get("/-/metadata.json") assert response2.json() == expected_metadata diff --git a/tests/test_auth.py b/tests/test_auth.py index 5868a21c..8e83d397 100644 --- a/tests/test_auth.py +++ b/tests/test_auth.py @@ -294,7 +294,7 @@ async def test_auth_with_dstok_token(ds_client, scenario, should_work): try: if should_work: data = response.json() - assert data.keys() == {"actor"} + assert data.keys() == {"ok", "actor"} actor = data["actor"] expected_keys = {"id", "token"} if scenario != "valid_unlimited_token": @@ -305,7 +305,7 @@ async def test_auth_with_dstok_token(ds_client, scenario, should_work): if scenario != "valid_unlimited_token": assert isinstance(actor["token_expires"], int) else: - assert response.json() == {"actor": None} + assert response.json() == {"ok": True, "actor": None} finally: ds_client.ds._settings["allow_signed_tokens"] = True @@ -337,10 +337,10 @@ def test_cli_create_token(app_client, expires): } if expires and expires > 0: expected_actor["token_expires"] = details["t"] + expires - assert response.json == {"actor": expected_actor} + assert response.json == {"ok": True, "actor": expected_actor} else: expected_actor = None - assert response.json == {"actor": expected_actor} + assert response.json == {"ok": True, "actor": expected_actor} @pytest.mark.asyncio diff --git a/tests/test_autocomplete.py b/tests/test_autocomplete.py index 76b9c902..194fcf01 100644 --- a/tests/test_autocomplete.py +++ b/tests/test_autocomplete.py @@ -25,13 +25,14 @@ async def test_autocomplete_single_pk_exact_match_and_label_order(): assert response.status_code == 200 assert response.json() == { + "ok": True, "rows": [ {"pks": {"id": 2}, "label": "Longer non-label pk match"}, {"pks": {"id": 20}, "label": "2"}, {"pks": {"id": 21}, "label": "22"}, {"pks": {"id": 3}, "label": "A label containing 2"}, {"pks": {"id": 200}, "label": "A"}, - ] + ], } @@ -52,12 +53,12 @@ async def test_autocomplete_blank_q_returns_no_results(): response = await ds.client.get("/autocomplete_blank/people/-/autocomplete?q=") assert response.status_code == 200 - assert response.json() == {"rows": []} + assert response.json() == {"ok": True, "rows": []} response = await ds.client.get("/autocomplete_blank/people/-/autocomplete") assert response.status_code == 200 - assert response.json() == {"rows": []} + assert response.json() == {"ok": True, "rows": []} @pytest.mark.asyncio @@ -81,11 +82,12 @@ async def test_autocomplete_initial_returns_latest_rows(): assert response.status_code == 200 assert response.json() == { + "ok": True, "rows": [ {"pks": {"id": 3}, "label": "Cleo"}, {"pks": {"id": 2}, "label": "Bob"}, {"pks": {"id": 1}, "label": "Alice"}, - ] + ], } response = await ds.client.get( @@ -94,11 +96,12 @@ async def test_autocomplete_initial_returns_latest_rows(): assert response.status_code == 200 assert response.json() == { + "ok": True, "rows": [ {"pks": {"id": 3}, "label": "Cleo"}, {"pks": {"id": 2}, "label": "Bob"}, {"pks": {"id": 1}, "label": "Alice"}, - ] + ], } @@ -121,9 +124,10 @@ async def test_autocomplete_escapes_like_characters(): assert response.status_code == 200 assert response.json() == { + "ok": True, "rows": [ {"pks": {"id": 1}, "label": "100% real"}, - ] + ], } @@ -149,11 +153,12 @@ async def test_autocomplete_compound_pk_searches_all_pk_columns(): assert response.status_code == 200 assert response.json() == { + "ok": True, "rows": [ {"pks": {"country": "mx", "code": "ca"}, "label": "Campeche"}, {"pks": {"country": "us", "code": "ca"}, "label": "California"}, {"pks": {"country": "ca", "code": "bc"}, "label": "British Columbia"}, - ] + ], } @@ -184,9 +189,10 @@ async def test_autocomplete_primary_key_called_label(): assert response.status_code == 200 assert response.json() == { + "ok": True, "rows": [ {"pks": {"label": "abc"}, "label": "Display value"}, - ] + ], } @@ -246,8 +252,9 @@ async def test_autocomplete_timeout_uses_prefix_fallback(monkeypatch): assert timeout_was_simulated data = response.json() assert data == { + "ok": True, "rows": [ {"pks": {"id": f"item-1999{i:02d}"}, "label": f"name 1999{i:02d}"} for i in range(10) - ] + ], } diff --git a/tests/test_cli_serve_get.py b/tests/test_cli_serve_get.py index dc852201..fe9416d6 100644 --- a/tests/test_cli_serve_get.py +++ b/tests/test_cli_serve_get.py @@ -95,7 +95,10 @@ def test_serve_with_get_and_token(): ], ) assert 0 == result2.exit_code, result2.output - assert json.loads(result2.output) == {"actor": {"id": "root", "token": "dstok"}} + assert json.loads(result2.output) == { + "ok": True, + "actor": {"id": "root", "token": "dstok"}, + } def test_serve_with_get_exit_code_for_error(): @@ -130,8 +133,9 @@ def test_serve_get_actor(): ) assert result.exit_code == 0 assert json.loads(result.output) == { + "ok": True, "actor": { "id": "root", "extra": "x", - } + }, } diff --git a/tests/test_docs.py b/tests/test_docs.py index 13b3a549..0bcb5e62 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -248,7 +248,7 @@ async def test_homepage(): async def test_actor_is_null(): ds = Datasette(memory=True) response = await ds.client.get("/-/actor.json") - assert response.json() == {"actor": None} + assert response.json() == {"ok": True, "actor": None} # -- end test_actor_is_null -- @@ -258,5 +258,5 @@ async def test_signed_cookie_actor(): ds = Datasette(memory=True) cookies = {"ds_actor": ds.client.actor_cookie({"id": "root"})} response = await ds.client.get("/-/actor.json", cookies=cookies) - assert response.json() == {"actor": {"id": "root"}} + assert response.json() == {"ok": True, "actor": {"id": "root"}} # -- end test_signed_cookie_actor -- diff --git a/tests/test_internals_datasette.py b/tests/test_internals_datasette.py index 2eaee3f9..1de394a5 100644 --- a/tests/test_internals_datasette.py +++ b/tests/test_internals_datasette.py @@ -188,7 +188,7 @@ async def test_num_sql_threads_zero(): await db.execute_write("create table t(id integer primary key)") await db.execute_write("insert into t (id) values (1)") response = await ds.client.get("/-/threads.json") - assert response.json() == {"num_threads": 0, "threads": []} + assert response.json() == {"ok": True, "num_threads": 0, "threads": []} response2 = await ds.client.get("/test_num_sql_threads_zero/t.json?_shape=array") assert response2.json() == [{"id": 1}] diff --git a/tests/test_internals_datasette_client.py b/tests/test_internals_datasette_client.py index 543077a5..e9aaaae8 100644 --- a/tests/test_internals_datasette_client.py +++ b/tests/test_internals_datasette_client.py @@ -318,7 +318,7 @@ async def test_actor_parameter_sets_cookie(datasette): """Passing actor= should sign a ds_actor cookie and authenticate the request.""" response = await datasette.client.get("/-/actor.json", actor={"id": "root"}) assert response.status_code == 200 - assert response.json() == {"actor": {"id": "root"}} + assert response.json() == {"ok": True, "actor": {"id": "root"}} @pytest.mark.asyncio @@ -327,7 +327,7 @@ async def test_actor_parameter_works_with_request_method(datasette): "GET", "/-/actor.json", actor={"id": "root"} ) assert response.status_code == 200 - assert response.json() == {"actor": {"id": "root"}} + assert response.json() == {"ok": True, "actor": {"id": "root"}} @pytest.mark.asyncio @@ -362,7 +362,7 @@ async def test_actor_parameter_merges_with_other_cookies(datasette): cookies={"unrelated": "value"}, ) assert response.status_code == 200 - assert response.json() == {"actor": {"id": "root"}} + assert response.json() == {"ok": True, "actor": {"id": "root"}} @pytest.mark.asyncio diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 8323fe92..7d99213b 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -739,6 +739,7 @@ async def test_actor_restricted_permissions( "path": expected_path, } expected = { + "ok": True, "action": permission, "allowed": expected_result, "resource": expected_resource, @@ -1115,7 +1116,7 @@ def test_cli_create_token(options, expected): ], ) assert 0 == result2.exit_code, result2.output - assert json.loads(result2.output) == {"actor": expected} + assert json.loads(result2.output) == {"ok": True, "actor": expected} _visible_tables_re = re.compile(r">\/((\w+)\/(\w+))\.json<\/a> - Get rows for") diff --git a/tests/test_plugins.py b/tests/test_plugins.py index da14c714..59b1c0bf 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -783,9 +783,13 @@ async def test_hook_permission_resources_sql(): @pytest.mark.asyncio async def test_actor_json(ds_client): - assert (await ds_client.get("/-/actor.json")).json() == {"actor": None} + assert (await ds_client.get("/-/actor.json")).json() == { + "ok": True, + "actor": None, + } assert (await ds_client.get("/-/actor.json?_bot2=1")).json() == { - "actor": {"id": "bot2", "1+1": 2} + "ok": True, + "actor": {"id": "bot2", "1+1": 2}, } diff --git a/tests/test_success_envelope.py b/tests/test_success_envelope.py new file mode 100644 index 00000000..48407660 --- /dev/null +++ b/tests/test_success_envelope.py @@ -0,0 +1,78 @@ +""" +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.) +""" + +import pytest +from datasette.app import Datasette +from datasette.utils import sqlite3 + + +@pytest.fixture +def ds_envelope(tmp_path_factory): + db_directory = tmp_path_factory.mktemp("dbs") + db_path = str(db_directory / "data.db") + conn = sqlite3.connect(db_path) + conn.execute("vacuum") + conn.execute("create table docs (id integer primary key, title text)") + conn.close() + ds = Datasette([db_path]) + ds.root_enabled = True + yield ds + ds.close() + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path", + ( + "/.json", + "/-/.json", + "/-/versions.json", + "/-/settings.json", + "/-/config.json", + "/-/threads.json", + "/-/actor.json", + "/-/jump.json", + "/-/schema.json", + "/fixtures/-/schema.json", + "/fixtures/facetable/-/schema.json", + "/-/allowed.json?action=view-instance", + "/fixtures/facet_cities/-/autocomplete?_initial=1", + ), +) +async def test_success_object_has_ok_true(ds_client, path): + response = await ds_client.get(path) + assert response.status_code == 200 + data = response.json() + assert isinstance(data, dict) + assert data["ok"] is True + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "path", + ( + "/-/rules.json?action=view-instance", + "/-/check.json?action=view-instance", + ), +) +async def test_permission_debug_success_has_ok_true(ds_envelope, path): + response = await ds_envelope.client.get(path, actor={"id": "root"}) + assert response.status_code == 200 + data = response.json() + assert data["ok"] is True + + +@pytest.mark.asyncio +async def test_permissions_post_success_has_ok_true(ds_envelope): + response = await ds_envelope.client.post( + "/-/permissions", + data={"actor": '{"id": "root"}', "permission": "view-instance"}, + actor={"id": "root"}, + ) + assert response.status_code == 200 + assert response.json()["ok"] is True