Redesign and document extras mechanism to cover rows and queries in addition to tables

Merge PR #2769

tests/test_api.py

@@ -383,7 +383,7 @@ async def test_row_strange_table_name(ds_client):
 @pytest.mark.asyncio
 async def test_row_foreign_key_tables(ds_client):
     response = await ds_client.get(
-        "/fixtures/simple_primary_key/1.json?_extras=foreign_key_tables"
+        "/fixtures/simple_primary_key/1.json?_extra=foreign_key_tables"
     )
     assert response.status_code == 200
     # Foreign keys are sorted by (other_table, column, other_column)
@@ -426,6 +426,28 @@ async def test_row_foreign_key_tables(ds_client):
     ]
 
 
+@pytest.mark.asyncio
+async def test_row_extras(ds_client):
+    response = await ds_client.get(
+        "/fixtures/simple_primary_key/1.json?_extra=database,table,primary_keys,query,request,debug,foreign_key_tables"
+    )
+    assert response.status_code == 200
+    data = response.json()
+    assert data["database"] == "fixtures"
+    assert data["table"] == "simple_primary_key"
+    assert data["primary_keys"] == ["id"]
+    assert data["query"]["sql"] == 'select * from simple_primary_key where "id"=:p0'
+    assert data["query"]["params"] == {"p0": "1"}
+    assert data["request"]["path"] == "/fixtures/simple_primary_key/1.json"
+    assert data["debug"]["url_vars"] == {
+        "database": "fixtures",
+        "table": "simple_primary_key",
+        "pks": "1",
+        "format": "json",
+    }
+    assert len(data["foreign_key_tables"]) == 5
+
+
 @pytest.mark.asyncio
 async def test_row_extra_render_cell():
     """Test that _extra=render_cell returns rendered HTML from render_cell plugin hook on row pages"""

tests/test_docs.py

@@ -112,6 +112,51 @@ def test_table_filters_are_documented(documented_table_filters, subtests):
             assert f.key in documented_table_filters
 
 
+def test_table_extra_examples_are_documented():
+    from datasette.views.table_extras import CountExtra
+
+    assert CountExtra.example.path == "/fixtures/facetable.json?_extra=count"
+    content = (docs_path / "json_api.rst").read_text()
+    section = content.split(".. _json_api_extra:")[-1].split(".. _table_arguments:")[0]
+    assert "GET /fixtures/facetable.json?_extra=count" in section
+    assert ".. code-block:: json" in section
+
+
+def test_render_cell_extra_example_explains_row_and_column_mapping():
+    content = (docs_path / "json_api.rst").read_text()
+    section = content.split("``render_cell``")[-1].split("``query``")[0]
+    assert "same order as the ``rows`` array" in section
+    assert '"rows": [' in section
+    assert '"render_cell": [' in section
+
+
+def test_debug_and_request_extra_examples_are_documented():
+    content = (docs_path / "json_api.rst").read_text()
+    section = content.split("Table JSON responses")[-1].split("Row JSON responses")[0]
+
+    debug_section = section.split("``debug``")[-1].split("``request``")[0]
+    assert "GET /fixtures/facetable.json?_extra=debug" in debug_section
+    assert '"url_vars": {' in debug_section
+
+    request_section = section.split("``request``")[-1].split("``query``")[0]
+    assert "GET /fixtures/facetable.json?_extra=request" in request_section
+    assert '"full_path":' in request_section
+
+
+def test_row_and_query_extra_sections_are_documented():
+    content = (docs_path / "json_api.rst").read_text()
+    assert "Row JSON responses" in content
+    assert (
+        "``GET /fixtures/simple_primary_key/1.json?_extra=foreign_key_tables``"
+        in content
+    )
+    assert "Query JSON responses" in content
+    assert "``GET /fixtures/-/query.json?sql=select+1+as+one&_extra=query``" in content
+    assert (
+        "``GET /fixtures/neighborhood_search.json?text=town&_extra=query``" in content
+    )
+
+
 @pytest.fixture(scope="session")
 def documented_labels():
     labels = set()

tests/test_extras.py

@@ -0,0 +1,65 @@
+import asyncio
+
+import pytest
+
+from datasette.extras import Extra, ExtraRegistry, ExtraScope
+
+
+class SlowValueExtra(Extra):
+    description = "Returns context['value'], optionally slowly"
+    scopes = {ExtraScope.TABLE}
+
+    async def resolve(self, context):
+        if context["slow"]:
+            await asyncio.sleep(0.05)
+        return context["value"]
+
+
+class DependentExtra(Extra):
+    description = "Depends on slow_value"
+    scopes = {ExtraScope.TABLE}
+
+    async def resolve(self, context, slow_value):
+        return slow_value + 1
+
+
+def test_registry_is_built_once_per_scope():
+    registry = ExtraRegistry([SlowValueExtra, DependentExtra])
+    first = registry._registry_for_scope(ExtraScope.TABLE)
+    second = registry._registry_for_scope(ExtraScope.TABLE)
+    assert first is second
+
+
+@pytest.mark.asyncio
+async def test_concurrent_resolves_do_not_share_state():
+    # The asyncinject registry is shared across requests - resolved values
+    # must not leak between concurrent resolve() calls with different contexts
+    registry = ExtraRegistry([SlowValueExtra, DependentExtra])
+    slow, fast = await asyncio.gather(
+        registry.resolve(
+            {"slow_value", "dependent"},
+            {"value": 100, "slow": True},
+            ExtraScope.TABLE,
+        ),
+        registry.resolve(
+            {"slow_value", "dependent"},
+            {"value": 200, "slow": False},
+            ExtraScope.TABLE,
+        ),
+    )
+    assert slow == {"slow_value": 100, "dependent": 101}
+    assert fast == {"slow_value": 200, "dependent": 201}
+
+
+@pytest.mark.asyncio
+async def test_table_row_and_query_scopes_use_separate_registries():
+    from datasette.views.table_extras import table_extra_registry
+
+    registries = {
+        scope: table_extra_registry._registry_for_scope(scope) for scope in ExtraScope
+    }
+    assert len(set(map(id, registries.values()))) == 3
+    # Scope-specific extras only registered where they belong
+    assert "count" in registries[ExtraScope.TABLE]._registry
+    assert "count" not in registries[ExtraScope.QUERY]._registry
+    assert "foreign_key_tables" in registries[ExtraScope.ROW]._registry

tests/test_table_api.py

@@ -68,6 +68,134 @@ async def test_table_shape_arrayfirst(ds_client):
     ]
 
 
+@pytest.mark.asyncio
+async def test_query_extras_for_arbitrary_sql(ds_client):
+    response = await ds_client.get(
+        "/fixtures/-/query.json?"
+        + urllib.parse.urlencode(
+            {
+                "sql": "select 1 as one",
+                "_extra": "columns,database,query,request,debug",
+            }
+        )
+    )
+    assert response.status_code == 200
+    data = response.json()
+    assert data["rows"] == [{"one": 1}]
+    assert data["columns"] == ["one"]
+    assert data["database"] == "fixtures"
+    assert data["query"]["sql"] == "select 1 as one"
+    assert data["request"]["path"] == "/fixtures/-/query.json"
+    assert data["debug"]["url_vars"] == {
+        "database": "fixtures",
+        "format": "json",
+    }
+
+
+@pytest.mark.asyncio
+async def test_query_extras_for_stored_query(ds_client):
+    response = await ds_client.get(
+        "/fixtures/neighborhood_search.json?"
+        + urllib.parse.urlencode(
+            {
+                "text": "town",
+                "_extra": "columns,database,query,request,debug",
+            }
+        )
+    )
+    assert response.status_code == 200
+    data = response.json()
+    assert data["columns"] == ["_neighborhood", "name", "state"]
+    assert data["database"] == "fixtures"
+    assert data["query"]["sql"].strip().startswith("select _neighborhood")
+    assert data["query"]["params"]["text"] == "town"
+    assert data["request"]["path"] == "/fixtures/neighborhood_search.json"
+    assert data["debug"]["url_vars"] == {
+        "database": "fixtures",
+        "table": "neighborhood_search",
+        "format": "json",
+    }
+
+
+@pytest.mark.parametrize("extra", ["filters", "actions", "display_rows"])
+@pytest.mark.asyncio
+async def test_html_only_extras_are_not_available_via_json(ds_client, extra):
+    # These extras exist for the HTML view; their values are not JSON
+    # serializable so they are internal, not part of the JSON API
+    response = await ds_client.get(f"/fixtures/facetable.json?_extra={extra}")
+    assert response.status_code == 200
+    assert extra not in response.json()
+
+
+@pytest.mark.asyncio
+async def test_html_only_extras_are_not_advertised(ds_client):
+    response = await ds_client.get("/fixtures/facetable.json?_extra=extras")
+    assert response.status_code == 200
+    names = {e["name"] for e in response.json()["extras"]}
+    assert {"filters", "actions", "display_rows"}.isdisjoint(names)
+
+
+def test_query_extra_private_for_arbitrary_sql():
+    with make_app_client(config={"allow_sql": {"id": "root"}}) as client:
+        cookies = {"ds_actor": client.actor_cookie({"id": "root"})}
+        response = client.get(
+            "/fixtures/-/query.json?sql=select+1+as+one&_extra=private",
+            cookies=cookies,
+        )
+        assert response.status == 200
+        assert response.json["private"] is True
+        # Anonymous users cannot execute SQL at all here
+        anon = client.get("/fixtures/-/query.json?sql=select+1+as+one")
+        assert anon.status == 403
+
+
+def test_query_extra_query_reports_bound_params():
+    config = {
+        "databases": {
+            "fixtures": {
+                "queries": {
+                    "declared_params": {
+                        "sql": "select 1 as one",
+                        "params": ["foo"],
+                    },
+                    "magic_host": {
+                        "sql": "select :_header_host as h",
+                    },
+                }
+            }
+        }
+    }
+    with make_app_client(config=config) as client:
+        # Declared parameters are reported even when the regex cannot find them
+        response = client.get("/fixtures/declared_params.json?foo=bar&_extra=query")
+        assert response.status == 200
+        assert response.json["query"]["params"] == {"foo": "bar"}
+        # Magic parameters are bound internally and should not be reported,
+        # especially not as a value taken from the querystring
+        response = client.get(
+            "/fixtures/magic_host.json?_extra=query&_header_host=spoofed"
+        )
+        assert response.status == 200
+        assert response.json["rows"] == [{"h": "localhost"}]
+        assert response.json["query"]["params"] == {}
+
+
+def test_query_extra_query_does_not_echo_querystring_without_sql():
+    with make_app_client() as client:
+        response = client.get("/fixtures/-/query.json?_extra=query&foo=bar")
+        assert response.status == 200
+        assert response.json["query"]["params"] == {}
+
+
+def test_query_extra_private_false_when_sql_is_public():
+    with make_app_client() as client:
+        response = client.get(
+            "/fixtures/-/query.json?sql=select+1+as+one&_extra=private"
+        )
+        assert response.status == 200
+        assert response.json["private"] is False
+
+
 @pytest.mark.asyncio
 async def test_table_shape_objects(ds_client):
     response = await ds_client.get("/fixtures/simple_primary_key.json?_shape=objects")
@@ -1376,6 +1504,17 @@ async def test_table_extras(ds_client, extra, expected_json):
     assert response.json() == expected_json
 
 
+@pytest.mark.asyncio
+async def test_table_extra_columns_can_be_comma_separated(ds_client):
+    response = await ds_client.get(
+        "/fixtures/primary_key_multiple_columns.json?_extra=columns,count"
+    )
+    assert response.status_code == 200
+    data = response.json()
+    assert data["columns"] == ["id", "content", "content2"]
+    assert data["count"] == 1
+
+
 @pytest.mark.asyncio
 async def test_extra_render_cell():
     """Test that _extra=render_cell returns rendered HTML from render_cell plugin hook"""