diff --git a/datasette/app.py b/datasette/app.py index 1f2883b7..e9337f5e 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -922,9 +922,7 @@ class Datasette: return self._app_css_hash async def get_canned_queries(self, database_name, actor): - queries = ( - ((self.config or {}).get("databases") or {}).get(database_name) or {} - ).get("queries") or {} + queries = {} for more_queries in pm.hook.canned_queries( datasette=self, database=database_name, diff --git a/datasette/default_permissions.py b/datasette/default_permissions.py index fe8db31e..e03f8e87 100644 --- a/datasette/default_permissions.py +++ b/datasette/default_permissions.py @@ -50,6 +50,7 @@ async def permission_resources_sql(datasette, actor, action): "view-database", "view-database-download", "view-table", + "view-query", "execute-sql", } if action in default_allow_actions: @@ -335,3 +336,12 @@ def skip_csrf(scope): headers = scope.get("headers") or {} if dict(headers).get(b"content-type") == b"application/json": return True + + +@hookimpl +def canned_queries(datasette, database, actor): + """Return canned queries from datasette configuration.""" + queries = ( + ((datasette.config or {}).get("databases") or {}).get(database) or {} + ).get("queries") or {} + return queries diff --git a/datasette/resources.py b/datasette/resources.py index f1cff82c..847f1686 100644 --- a/datasette/resources.py +++ b/datasette/resources.py @@ -13,7 +13,7 @@ class InstanceResource(Resource): super().__init__(parent=None, child=None) @classmethod - def resources_sql(cls) -> str: + async def resources_sql(cls, datasette) -> str: return "SELECT NULL AS parent, NULL AS child" @@ -27,7 +27,7 @@ class DatabaseResource(Resource): super().__init__(parent=database, child=None) @classmethod - def resources_sql(cls) -> str: + async def resources_sql(cls, datasette) -> str: return """ SELECT database_name AS parent, NULL AS child FROM catalog_databases @@ -44,7 +44,7 @@ class TableResource(Resource): super().__init__(parent=database, child=table) @classmethod - def resources_sql(cls) -> str: + async def resources_sql(cls, datasette) -> str: return """ SELECT database_name AS parent, table_name AS child FROM catalog_tables @@ -64,6 +64,41 @@ class QueryResource(Resource): super().__init__(parent=database, child=query) @classmethod - def resources_sql(cls) -> str: - # TODO: Need catalog for queries - return "SELECT NULL AS parent, NULL AS child WHERE 0" + async def resources_sql(cls, datasette) -> str: + from datasette.plugins import pm + from datasette.utils import await_me_maybe + + # Get all databases from catalog + db = datasette.get_internal_database() + result = await db.execute("SELECT database_name FROM catalog_databases") + databases = [row[0] for row in result.rows] + + # Gather all canned queries from all databases + query_pairs = [] + for database_name in databases: + # Call the hook to get queries (including from config via default plugin) + for queries_result in pm.hook.canned_queries( + datasette=datasette, + database=database_name, + actor=None, # Get ALL queries for resource enumeration + ): + queries = await await_me_maybe(queries_result) + if queries: + for query_name in queries.keys(): + query_pairs.append((database_name, query_name)) + + # Build SQL + if not query_pairs: + return "SELECT NULL AS parent, NULL AS child WHERE 0" + + # Generate UNION ALL query + selects = [] + for db_name, query_name in query_pairs: + # Escape single quotes by doubling them + db_escaped = db_name.replace("'", "''") + query_escaped = query_name.replace("'", "''") + selects.append( + f"SELECT '{db_escaped}' AS parent, '{query_escaped}' AS child" + ) + + return " UNION ALL ".join(selects) diff --git a/datasette/utils/actions_sql.py b/datasette/utils/actions_sql.py index dfd0867f..6807a728 100644 --- a/datasette/utils/actions_sql.py +++ b/datasette/utils/actions_sql.py @@ -177,7 +177,7 @@ async def _build_single_action_sql( raise ValueError(f"Unknown action: {action}") # Get base resources SQL from the resource class - base_resources_sql = action_obj.resource_class.resources_sql() + base_resources_sql = await action_obj.resource_class.resources_sql(datasette) # Get all permission rule fragments from plugins via the hook rule_results = pm.hook.permission_resources_sql( diff --git a/tests/test_api.py b/tests/test_api.py index adaf3e73..84b33a09 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -898,9 +898,6 @@ async def test_json_columns(ds_client, extra_args, expected): assert response.json() == expected -@pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" -) def test_config_cache_size(app_client_larger_cache_size): response = app_client_larger_cache_size.get("/fixtures/pragma_cache_size.json") assert response.json["rows"] == [{"cache_size": -2500}] diff --git a/tests/test_canned_queries.py b/tests/test_canned_queries.py index ee849b36..c84c8cdb 100644 --- a/tests/test_canned_queries.py +++ b/tests/test_canned_queries.py @@ -4,11 +4,6 @@ import pytest import re from .fixtures import make_app_client, app_client -# Mark entire module as xfail since view-query permission not yet migrated, refs #2534 -pytestmark = pytest.mark.xfail( - reason="view-query permission not yet migrated to new permission system, refs #2534" -) - @pytest.fixture def canned_write_client(tmpdir): diff --git a/tests/test_html.py b/tests/test_html.py index 46725afc..a6dc52c0 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -135,9 +135,6 @@ def test_not_allowed_methods(): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not displayed due to view-query permission, refs #2510" -) async def test_database_page(ds_client): response = await ds_client.get("/fixtures") soup = Soup(response.text, "html.parser") @@ -263,10 +260,9 @@ def test_query_page_truncates(): "/fixtures/simple_primary_key", ["table", "db-fixtures", "table-simple_primary_key"], ), - pytest.param( + ( "/fixtures/neighborhood_search", ["query", "db-fixtures", "query-neighborhood_search"], - marks=pytest.mark.xfail(reason="Canned queries not accessible, refs #2510"), ), ( "/fixtures/table~2Fwith~2Fslashes~2Ecsv", @@ -599,9 +595,6 @@ async def test_404_content_type(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_canned_query_default_title(ds_client): response = await ds_client.get("/fixtures/magic_parameters") assert response.status_code == 200 @@ -610,9 +603,6 @@ async def test_canned_query_default_title(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_canned_query_with_custom_metadata(ds_client): response = await ds_client.get("/fixtures/neighborhood_search?text=town") assert response.status_code == 200 @@ -675,9 +665,6 @@ async def test_show_hide_sql_query(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_canned_query_with_hide_has_no_hidden_sql(ds_client): # For a canned query the show/hide should NOT have a hidden SQL field # https://github.com/simonw/datasette/issues/1411 @@ -689,9 +676,6 @@ async def test_canned_query_with_hide_has_no_hidden_sql(ds_client): ] == [(hidden["name"], hidden["value"]) for hidden in hiddens] -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) @pytest.mark.parametrize( "hide_sql,querystring,expected_hidden,expected_show_hide_link,expected_show_hide_text", ( @@ -941,9 +925,6 @@ def test_base_url_affects_metadata_extra_css_urls(app_client_base_url_prefix): ("/fixtures/magic_parameters", None), ], ) -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_edit_sql_link_on_canned_queries(ds_client, path, expected): response = await ds_client.get(path) assert response.status_code == 200 @@ -959,9 +940,6 @@ async def test_edit_sql_link_on_canned_queries(ds_client, path, expected): [ pytest.param( True, - marks=pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" - ), ), False, ], @@ -1057,10 +1035,9 @@ async def test_trace_correctly_escaped(ds_client): "http://localhost/fixtures/-/query.json?sql=select+*+from+facetable", ), # Canned query page - pytest.param( + ( "/fixtures/neighborhood_search?text=town", "http://localhost/fixtures/neighborhood_search.json?text=town", - marks=pytest.mark.xfail(reason="Canned queries not accessible, refs #2510"), ), # /-/ pages ( @@ -1180,7 +1157,7 @@ async def test_database_color(ds_client): "/fixtures", "/fixtures/facetable", "/fixtures/paginated_view", - # "/fixtures/pragma_cache_size", # Canned query - skipped due to view-query not migrated, refs #2510 + "/fixtures/pragma_cache_size", ): response = await ds_client.get(path) assert any( diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 0bf28e2e..e40d8c99 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -234,7 +234,6 @@ def test_table_list_respects_view_table(): assert html_fragment in auth_response.text -@pytest.mark.xfail(reason="view-query not yet migrated to new permission system") @pytest.mark.parametrize( "allow,expected_anon,expected_auth", [ @@ -365,9 +364,6 @@ def test_query_list_respects_view_query(): ("view-database", "fixtures"), ("view-query", ("fixtures", "neighborhood_search")), ], - marks=pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" - ), ), ], ) @@ -593,9 +589,6 @@ def test_permissions_cascade(cascade_app_client, path, permissions, expected_sta cascade_app_client.ds.config = previous_config -@pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" -) def test_padlocks_on_database_page(cascade_app_client): config = { "databases": { diff --git a/tests/test_plugins.py b/tests/test_plugins.py index acdf18f5..205c04bd 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -499,9 +499,6 @@ async def test_hook_register_output_renderer_all_parameters(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" -) async def test_hook_register_output_renderer_custom_status_code(ds_client): response = await ds_client.get( "/fixtures/pragma_cache_size.testall?status_code=202" @@ -510,9 +507,6 @@ async def test_hook_register_output_renderer_custom_status_code(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" -) async def test_hook_register_output_renderer_custom_content_type(ds_client): response = await ds_client.get( "/fixtures/pragma_cache_size.testall?content_type=text/blah" @@ -521,9 +515,6 @@ async def test_hook_register_output_renderer_custom_content_type(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" -) async def test_hook_register_output_renderer_custom_headers(ds_client): response = await ds_client.get( "/fixtures/pragma_cache_size.testall?header=x-wow:1&header=x-gosh:2" @@ -854,9 +845,6 @@ async def test_hook_startup(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_hook_canned_queries(ds_client): queries = (await ds_client.get("/fixtures.json")).json()["queries"] queries_by_name = {q["name"]: q for q in queries} @@ -873,34 +861,24 @@ async def test_hook_canned_queries(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_hook_canned_queries_non_async(ds_client): response = await ds_client.get("/fixtures/from_hook.json?_shape=array") assert [{"1": 1, "actor_id": "null"}] == response.json() @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_hook_canned_queries_async(ds_client): response = await ds_client.get("/fixtures/from_async_hook.json?_shape=array") assert [{"2": 2}] == response.json() @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_hook_canned_queries_actor(ds_client): assert ( await ds_client.get("/fixtures/from_hook.json?_bot=1&_shape=array") ).json() == [{"1": 1, "actor_id": "bot"}] -@pytest.mark.xfail(reason="Magic parameters used with canned queries, refs #2510") def test_hook_register_magic_parameters(restore_working_directory): with make_app_client( extra_databases={"data.db": "create table logs (line text)"}, @@ -1048,9 +1026,6 @@ def get_actions_links(html): pytest.param( "/fixtures/pragma_cache_size", "/fixtures/-/query?sql=explain+PRAGMA+cache_size%3B", - marks=pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" - ), ), # Don't attempt to explain an explain ("/fixtures/-/query?sql=explain+select+1", None), @@ -1558,9 +1533,6 @@ async def test_hook_top_query(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not yet migrated to new permission system, refs #2510" -) async def test_hook_top_canned_query(ds_client): try: pm.register(SlotPlugin(), name="SlotPlugin") diff --git a/tests/test_table_api.py b/tests/test_table_api.py index a7dcb8c6..0b722519 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -1147,9 +1147,6 @@ async def test_infinity_returned_as_invalid_json_if_requested(ds_client): @pytest.mark.asyncio -@pytest.mark.xfail( - reason="Canned queries not accessible due to view-query permission not migrated, refs #2510" -) async def test_custom_query_with_unicode_characters(ds_client): # /fixtures/𝐜𝐢𝐭𝐢𝐞𝐬.json response = await ds_client.get(