From a23c2aee006bea5f50c13e687dac77c66df1888b Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Mon, 15 Jul 2024 10:33:51 -0700 Subject: [PATCH] Introduce new `/$DB/-/query` endpoint, soft replaces `/$DB?sql=...` (#2363) * Introduce new default /$DB/-/query endpoint * Fix a lot of tests * Update pyodide test to use query endpoint * Link to /fixtures/-/query in a few places * Documentation for QueryView --------- Co-authored-by: Simon Willison --- datasette/app.py | 6 ++- datasette/templates/database.html | 4 +- datasette/views/database.py | 8 ++++ docs/pages.rst | 15 +++++++ test-in-pyodide-with-shot-scraper.sh | 4 +- tests/plugins/my_plugin.py | 1 + tests/test_api.py | 38 ++++++++++++------ tests/test_api_write.py | 12 ++++-- tests/test_canned_queries.py | 2 +- tests/test_cli.py | 8 ++-- tests/test_cli_serve_get.py | 2 +- tests/test_crossdb.py | 6 ++- tests/test_csv.py | 10 ++--- tests/test_html.py | 59 ++++++++++++++++------------ tests/test_load_extensions.py | 12 +++--- tests/test_messages.py | 4 +- tests/test_permissions.py | 8 ++-- tests/test_plugins.py | 22 ++++++----- tests/test_routes.py | 2 +- tests/test_table_api.py | 4 +- tests/test_table_html.py | 4 +- 21 files changed, 148 insertions(+), 83 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 1c5d6a3727..5b8f910ce3 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -37,7 +37,7 @@ from .events import Event from .views import Context from .views.base import ureg -from .views.database import database_download, DatabaseView, TableCreateView +from .views.database import database_download, DatabaseView, TableCreateView, QueryView from .views.index import IndexView from .views.special import ( JsonDataView, @@ -1578,6 +1578,10 @@ def add_route(view, regex): r"/(?P[^\/\.]+)(\.(?P\w+))?$", ) add_route(TableCreateView.as_view(self), r"/(?P[^\/\.]+)/-/create$") + add_route( + wrap_view(QueryView, self), + r"/(?P[^\/\.]+)/-/query(\.(?P\w+))?$", + ) add_route( wrap_view(table_view, self), r"/(?P[^\/\.]+)/(?P[^\/\.]+)(\.(?P\w+))?$", diff --git a/datasette/templates/database.html b/datasette/templates/database.html index 4a4a05cbf2..f921bc2dbc 100644 --- a/datasette/templates/database.html +++ b/datasette/templates/database.html @@ -21,7 +21,7 @@

{{ metadata.title or database }}{% if private %} 🔒{% endif %}

{% block description_source_license %}{% include "_description_source_license.html" %}{% endblock %} {% if allow_execute_sql %} -
+

Custom SQL query

@@ -36,7 +36,7 @@

Custom SQL query

The following databases are attached to this connection, and can be used for cross-database joins:

    {% for db_name in attached_databases %} -
  • {{ db_name }} - tables
    • +
  • {{ db_name }} - tables
    • {% endfor %}
diff --git a/datasette/views/database.py b/datasette/views/database.py index 1d76c5e047..9ab061a106 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -58,6 +58,11 @@ async def get(self, request, datasette): sql = (request.args.get("sql") or "").strip() if sql: + redirect_url = "/" + request.url_vars.get("database") + "/-/query" + if request.url_vars.get("format"): + redirect_url += "." + request.url_vars.get("format") + redirect_url += "?" + request.query_string + return Response.redirect(redirect_url) return await QueryView()(request, datasette) if format_ not in ("html", "json"): @@ -433,6 +438,8 @@ async def post(self, request, datasette): async def get(self, request, datasette): from datasette.app import TableNotFound + await datasette.refresh_schemas() + db = await datasette.resolve_database(request) database = db.name @@ -686,6 +693,7 @@ async def fetch_data_for_csv(request, _next=None): if allow_execute_sql and is_validated_sql and ":_" not in sql: edit_sql_url = ( datasette.urls.database(database) + + "/-/query" + "?" + urlencode( { diff --git a/docs/pages.rst b/docs/pages.rst index 1c3e2c1e6e..239c9f8027 100644 --- a/docs/pages.rst +++ b/docs/pages.rst @@ -55,6 +55,21 @@ The following tables are hidden by default: - Tables relating to the inner workings of the SpatiaLite SQLite extension. - ``sqlite_stat`` tables used to store statistics used by the query optimizer. +.. _QueryView: + +Queries +======= + +The ``/database-name/-/query`` page can be used to execute an arbitrary SQL query against that database, if the :ref:`permissions_execute_sql` permission is enabled. This query is passed as the ``?sql=`` query string parameter. + +This means you can link directly to a query by constructing the following URL: + +``/database-name/-/query?sql=SELECT+*+FROM+table_name`` + +Each configured :ref:`canned query ` has its own page, at ``/database-name/query-name``. Viewing this page will execute the query and display the results. + +In both cases adding a ``.json`` extension to the URL will return the results as JSON. + .. _TableView: Table diff --git a/test-in-pyodide-with-shot-scraper.sh b/test-in-pyodide-with-shot-scraper.sh index 5845a0c3fd..4d1c496857 100755 --- a/test-in-pyodide-with-shot-scraper.sh +++ b/test-in-pyodide-with-shot-scraper.sh @@ -40,11 +40,11 @@ async () => { import setuptools from datasette.app import Datasette ds = Datasette(memory=True, settings={'num_sql_threads': 0}) - (await ds.client.get('/_memory.json?sql=select+55+as+itworks&_shape=array')).text + (await ds.client.get('/_memory/-/query.json?sql=select+55+as+itworks&_shape=array')).text \`); if (JSON.parse(output)[0].itworks != 55) { throw 'Got ' + output + ', expected itworks: 55'; } return 'Test passed!'; } -" \ No newline at end of file +" diff --git a/tests/plugins/my_plugin.py b/tests/plugins/my_plugin.py index 9ef10181d2..4ca4f989a0 100644 --- a/tests/plugins/my_plugin.py +++ b/tests/plugins/my_plugin.py @@ -411,6 +411,7 @@ def query_actions(datasette, database, query_name, sql): return [ { "href": datasette.urls.database(database) + + "/-/query" + "?" + urllib.parse.urlencode( { diff --git a/tests/test_api.py b/tests/test_api.py index bd7346776c..431ab5cee4 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -623,7 +623,7 @@ def test_no_files_uses_memory_database(app_client_no_files): } == response.json # Try that SQL query response = app_client_no_files.get( - "/_memory.json?sql=select+sqlite_version()&_shape=array" + "/_memory/-/query.json?sql=select+sqlite_version()&_shape=array" ) assert 1 == len(response.json) assert ["sqlite_version()"] == list(response.json[0].keys()) @@ -653,7 +653,7 @@ def test_database_page_for_database_with_dot_in_name(app_client_with_dot): @pytest.mark.asyncio async def test_custom_sql(ds_client): response = await ds_client.get( - "/fixtures.json?sql=select+content+from+simple_primary_key" + "/fixtures/-/query.json?sql=select+content+from+simple_primary_key", ) data = response.json() assert data == { @@ -670,7 +670,9 @@ async def test_custom_sql(ds_client): def test_sql_time_limit(app_client_shorter_time_limit): - response = app_client_shorter_time_limit.get("/fixtures.json?sql=select+sleep(0.5)") + response = app_client_shorter_time_limit.get( + "/fixtures/-/query.json?sql=select+sleep(0.5)", + ) assert 400 == response.status assert response.json == { "ok": False, @@ -691,16 +693,22 @@ def test_sql_time_limit(app_client_shorter_time_limit): @pytest.mark.asyncio async def test_custom_sql_time_limit(ds_client): - response = await ds_client.get("/fixtures.json?sql=select+sleep(0.01)") + response = await ds_client.get( + "/fixtures/-/query.json?sql=select+sleep(0.01)", + ) assert response.status_code == 200 - response = await ds_client.get("/fixtures.json?sql=select+sleep(0.01)&_timelimit=5") + response = await ds_client.get( + "/fixtures/-/query.json?sql=select+sleep(0.01)&_timelimit=5", + ) assert response.status_code == 400 assert response.json()["title"] == "SQL Interrupted" @pytest.mark.asyncio async def test_invalid_custom_sql(ds_client): - response = await ds_client.get("/fixtures.json?sql=.schema") + response = await ds_client.get( + "/fixtures/-/query.json?sql=.schema", + ) assert response.status_code == 400 assert response.json()["ok"] is False assert "Statement must be a SELECT" == response.json()["error"] @@ -883,9 +891,13 @@ async def test_json_columns(ds_client, extra_args, expected): select 1 as intval, "s" as strval, 0.5 as floatval, '{"foo": "bar"}' as jsonval """ - path = "/fixtures.json?" + urllib.parse.urlencode({"sql": sql, "_shape": "array"}) + path = "/fixtures/-/query.json?" + urllib.parse.urlencode( + {"sql": sql, "_shape": "array"} + ) path += extra_args - response = await ds_client.get(path) + response = await ds_client.get( + path, + ) assert response.json() == expected @@ -917,7 +929,7 @@ def test_config_force_https_urls(): ("/fixtures.json", 200), ("/fixtures/no_primary_key.json", 200), # A 400 invalid SQL query should still have the header: - ("/fixtures.json?sql=select+blah", 400), + ("/fixtures/-/query.json?sql=select+blah", 400), # Write APIs ("/fixtures/-/create", 405), ("/fixtures/facetable/-/insert", 405), @@ -930,7 +942,9 @@ def test_cors( path, status_code, ): - response = app_client_with_cors.get(path) + response = app_client_with_cors.get( + path, + ) assert response.status == status_code assert response.headers["Access-Control-Allow-Origin"] == "*" assert ( @@ -946,7 +960,9 @@ def test_cors( # should not have those headers - I'm using that fixture because # regular app_client doesn't have immutable fixtures.db which means # the test for /fixtures.db returns a 403 error - response = app_client_two_attached_databases_one_immutable.get(path) + response = app_client_two_attached_databases_one_immutable.get( + path, + ) assert response.status == status_code assert "Access-Control-Allow-Origin" not in response.headers assert "Access-Control-Allow-Headers" not in response.headers diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 94eb902b2b..b442113ba7 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -637,7 +637,9 @@ async def test_delete_row(ds_write, table, row_for_create, pks, delete_path): # Should be a single row assert ( await ds_write.client.get( - "/data.json?_shape=arrayfirst&sql=select+count(*)+from+{}".format(table) + "/data/-/query.json?_shape=arrayfirst&sql=select+count(*)+from+{}".format( + table + ) ) ).json() == [1] # Now delete the row @@ -645,7 +647,9 @@ async def test_delete_row(ds_write, table, row_for_create, pks, delete_path): # Special case for that rowid table delete_path = ( await ds_write.client.get( - "/data.json?_shape=arrayfirst&sql=select+rowid+from+{}".format(table) + "/data/-/query.json?_shape=arrayfirst&sql=select+rowid+from+{}".format( + table + ) ) ).json()[0] @@ -663,7 +667,9 @@ async def test_delete_row(ds_write, table, row_for_create, pks, delete_path): assert event.pks == str(delete_path).split(",") assert ( await ds_write.client.get( - "/data.json?_shape=arrayfirst&sql=select+count(*)+from+{}".format(table) + "/data/-/query.json?_shape=arrayfirst&sql=select+count(*)+from+{}".format( + table + ) ) ).json() == [0] diff --git a/tests/test_canned_queries.py b/tests/test_canned_queries.py index 69ed5ff904..d1ed06d183 100644 --- a/tests/test_canned_queries.py +++ b/tests/test_canned_queries.py @@ -412,7 +412,7 @@ def test_magic_parameters_csrf_json(magic_parameters_client, use_csrf, return_js def test_magic_parameters_cannot_be_used_in_arbitrary_queries(magic_parameters_client): response = magic_parameters_client.get( - "/data.json?sql=select+:_header_host&_shape=array" + "/data/-/query.json?sql=select+:_header_host&_shape=array" ) assert 400 == response.status assert response.json["error"].startswith("You did not supply a value for binding") diff --git a/tests/test_cli.py b/tests/test_cli.py index 486852cd4b..d53e0a5f20 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -250,7 +250,7 @@ def test_plugin_s_overwrite(): "--plugins-dir", plugins_dir, "--get", - "/_memory.json?sql=select+prepare_connection_args()", + "/_memory/-/query.json?sql=select+prepare_connection_args()", ], ) assert result.exit_code == 0, result.output @@ -265,7 +265,7 @@ def test_plugin_s_overwrite(): "--plugins-dir", plugins_dir, "--get", - "/_memory.json?sql=select+prepare_connection_args()", + "/_memory/-/query.json?sql=select+prepare_connection_args()", "-s", "plugins.name-of-plugin", "OVERRIDE", @@ -295,7 +295,7 @@ def test_setting_default_allow_sql(default_allow_sql): "default_allow_sql", "on" if default_allow_sql else "off", "--get", - "/_memory.json?sql=select+21&_shape=objects", + "/_memory/-/query.json?sql=select+21&_shape=objects", ], ) if default_allow_sql: @@ -309,7 +309,7 @@ def test_setting_default_allow_sql(default_allow_sql): def test_sql_errors_logged_to_stderr(): runner = CliRunner(mix_stderr=False) - result = runner.invoke(cli, ["--get", "/_memory.json?sql=select+blah"]) + result = runner.invoke(cli, ["--get", "/_memory/-/query.json?sql=select+blah"]) assert result.exit_code == 1 assert "sql = 'select blah', params = {}: no such column: blah\n" in result.stderr diff --git a/tests/test_cli_serve_get.py b/tests/test_cli_serve_get.py index 1088d9069c..513669a178 100644 --- a/tests/test_cli_serve_get.py +++ b/tests/test_cli_serve_get.py @@ -31,7 +31,7 @@ def startup(datasette): "--plugins-dir", str(plugins_dir), "--get", - "/_memory.json?sql=select+sqlite_version()", + "/_memory/-/query.json?sql=select+sqlite_version()", ], ) assert result.exit_code == 0, result.output diff --git a/tests/test_crossdb.py b/tests/test_crossdb.py index 01c5113041..58b81f705c 100644 --- a/tests/test_crossdb.py +++ b/tests/test_crossdb.py @@ -25,7 +25,8 @@ def test_crossdb_join(app_client_two_attached_databases_crossdb_enabled): fixtures.searchable """ response = app_client.get( - "/_memory.json?" + urllib.parse.urlencode({"sql": sql, "_shape": "array"}) + "/_memory/-/query.json?" + + urllib.parse.urlencode({"sql": sql, "_shape": "array"}) ) assert response.status == 200 assert response.json == [ @@ -67,9 +68,10 @@ def test_crossdb_attached_database_list_display( ): app_client = app_client_two_attached_databases_crossdb_enabled response = app_client.get("/_memory") + response2 = app_client.get("/") for fragment in ( "databases are attached to this connection", "
  • fixtures - ", - "
  • extra database - ", + '
  • extra database - json' in response.text - assert 'CSV' in response.text + assert 'json' in response.text + assert ( + 'CSV' + in response.text + ) @pytest.mark.asyncio async def test_query_parameter_form_fields(ds_client): - response = await ds_client.get("/fixtures?sql=select+:name") + response = await ds_client.get("/fixtures/-/query?sql=select+:name") assert response.status_code == 200 assert ( ' ' in response.text ) - response2 = await ds_client.get("/fixtures?sql=select+:name&name=hello") + response2 = await ds_client.get("/fixtures/-/query?sql=select+:name&name=hello") assert response2.status_code == 200 assert ( ' ' @@ -453,7 +458,9 @@ async def test_database_metadata(ds_client): @pytest.mark.asyncio async def test_database_metadata_with_custom_sql(ds_client): - response = await ds_client.get("/fixtures?sql=select+*+from+simple_primary_key") + response = await ds_client.get( + "/fixtures/-/query?sql=select+*+from+simple_primary_key" + ) assert response.status_code == 200 soup = Soup(response.text, "html.parser") # Page title should be the default @@ -591,7 +598,7 @@ async def test_canned_query_with_custom_metadata(ds_client): @pytest.mark.asyncio async def test_urlify_custom_queries(ds_client): - path = "/fixtures?" + urllib.parse.urlencode( + path = "/fixtures/-/query?" + urllib.parse.urlencode( {"sql": "select ('https://twitter.com/' || 'simonw') as user_url;"} ) response = await ds_client.get(path) @@ -609,7 +616,7 @@ async def test_urlify_custom_queries(ds_client): @pytest.mark.asyncio async def test_show_hide_sql_query(ds_client): - path = "/fixtures?" + urllib.parse.urlencode( + path = "/fixtures/-/query?" + urllib.parse.urlencode( {"sql": "select ('https://twitter.com/' || 'simonw') as user_url;"} ) response = await ds_client.get(path) @@ -696,15 +703,15 @@ def test_canned_query_show_hide_metadata_option( @pytest.mark.asyncio async def test_binary_data_display_in_query(ds_client): - response = await ds_client.get("/fixtures?sql=select+*+from+binary_data") + response = await ds_client.get("/fixtures/-/query?sql=select+*+from+binary_data") assert response.status_code == 200 table = Soup(response.content, "html.parser").find("table") expected_tds = [ [ - '
    • ' + '' ], [ - '' + '' ], [''], ] @@ -719,7 +726,7 @@ async def test_binary_data_display_in_query(ds_client): [ ("/fixtures/binary_data/1.blob?_blob_column=data", "binary_data-1-data.blob"), ( - "/fixtures.blob?sql=select+*+from+binary_data&_blob_column=data&_blob_hash=f3088978da8f9aea479ffc7f631370b968d2e855eeb172bea7f6c7a04262bb6d", + "/fixtures/-/query.blob?sql=select+*+from+binary_data&_blob_column=data&_blob_hash=f3088978da8f9aea479ffc7f631370b968d2e855eeb172bea7f6c7a04262bb6d", "data-f30889.blob", ), ], @@ -758,7 +765,7 @@ async def test_blob_download_invalid_messages(ds_client, path, expected_message) @pytest.mark.parametrize( "path", [ - "/fixtures?sql=select+*+from+[123_starts_with_digits]", + "/fixtures/-/query?sql=select+*+from+[123_starts_with_digits]", "/fixtures/123_starts_with_digits", ], ) @@ -771,7 +778,7 @@ async def test_zero_results(ds_client, path): @pytest.mark.asyncio async def test_query_error(ds_client): - response = await ds_client.get("/fixtures?sql=select+*+from+notatable") + response = await ds_client.get("/fixtures/-/query?sql=select+*+from+notatable") html = response.text assert '

    no such table: notatable

    ' in html assert '
    <Binary:\xa07\xa0bytes><Binary:\xa07\xa0bytes><Binary:\xa07\xa0bytes><Binary:\xa07\xa0bytes>\xa0