From 8f9509f00cceea8dc87403c28b2056db7b246ed4 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Mon, 22 Apr 2024 16:01:37 -0700 Subject: [PATCH 01/48] datasette, not self.ds, in internals documentation --- docs/internals.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/internals.rst b/docs/internals.rst index b57a289a07..795856594d 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -386,7 +386,7 @@ This is useful when you need to check multiple permissions at once. For example, .. code-block:: python - await self.ds.ensure_permissions( + await datasette.ensure_permissions( request.actor, [ ("view-table", (database, table)), @@ -420,7 +420,7 @@ This example checks if the user can access a specific table, and sets ``private` .. code-block:: python - visible, private = await self.ds.check_visibility( + visible, private = await datasette.check_visibility( request.actor, action="view-table", resource=(database, table), @@ -430,7 +430,7 @@ The following example runs three checks in a row, similar to :ref:`datasette_ens .. code-block:: python - visible, private = await self.ds.check_visibility( + visible, private = await datasette.check_visibility( request.actor, permissions=[ ("view-table", (database, table)), From e1bfab3fca22e4f8d06dcf419e945f13fc20aef1 Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Tue, 11 Jun 2024 09:33:23 -0700 Subject: [PATCH 02/48] Move Metadata to `--internal` database Refs: - https://github.com/simonw/datasette/pull/2343 - https://github.com/simonw/datasette/issues/2341 --- datasette/app.py | 210 +++++++++++++++++++++----------- datasette/default_menu_links.py | 4 - datasette/facets.py | 13 +- datasette/hookspecs.py | 5 - datasette/renderer.py | 1 - datasette/utils/internal_db.py | 37 ++++++ datasette/views/base.py | 6 +- datasette/views/database.py | 7 +- datasette/views/index.py | 10 +- datasette/views/row.py | 5 +- datasette/views/table.py | 38 ++++-- docs/codespell-ignore-words.txt | 3 +- docs/plugin_hooks.rst | 7 +- tests/test_api.py | 62 ++++++---- tests/test_cli.py | 4 +- tests/test_config_dir.py | 17 --- tests/test_facets.py | 9 +- tests/test_html.py | 14 +-- tests/test_permissions.py | 1 - tests/test_plugins.py | 41 +------ tests/test_routes.py | 2 - tests/test_table_html.py | 4 +- 22 files changed, 286 insertions(+), 214 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 23d21600c6..8020c5dac2 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -443,6 +443,37 @@ def __init__( self._root_token = secrets.token_hex(32) self.client = DatasetteClient(self) + async def apply_metadata_json(self): + # Apply any metadata entries from metadata.json to the internal tables + # step 1: top-level metadata + for key in self._metadata_local or {}: + if key == "databases": + continue + await self.set_instance_metadata(key, self._metadata_local[key]) + + # step 2: database-level metadata + for dbname, db in self._metadata_local.get("databases", {}).items(): + for key, value in db.items(): + if key == "tables": + continue + await self.set_database_metadata(dbname, key, value) + + # step 3: table-level metadata + for tablename, table in db.get("tables", {}).items(): + for key, value in table.items(): + if key == "columns": + continue + await self.set_resource_metadata(dbname, tablename, key, value) + + # step 4: column-level metadata (only descriptions in metadata.json) + for columnname, column_description in table.get("columns", {}).items(): + await self.set_column_metadata( + dbname, tablename, columnname, "description", column_description + ) + + # TODO(alex) is metadata.json was loaded in, and --internal is not memory, then log + # a warning to user that they should delete their metadata.json file + def get_jinja_environment(self, request: Request = None) -> Environment: environment = self._jinja_env if request: @@ -476,6 +507,7 @@ async def _refresh_schemas(self): internal_db = self.get_internal_database() if not self.internal_db_created: await init_internal_db(internal_db) + await self.apply_metadata_json() self.internal_db_created = True current_schema_versions = { row["database_name"]: row["schema_version"] @@ -646,57 +678,113 @@ def _metadata_recursive_update(self, orig, updated): orig[key] = upd_value return orig - def metadata(self, key=None, database=None, table=None, fallback=True): - """ - Looks up metadata, cascading backwards from specified level. - Returns None if metadata value is not found. - """ - assert not ( - database is None and table is not None - ), "Cannot call metadata() with table= specified but not database=" - metadata = {} + async def get_instance_metadata(self): + rows = await self.get_internal_database().execute( + """ + SELECT + key, + value + FROM datasette_metadata_instance_entries + """ + ) + return dict(rows) + + async def get_database_metadata(self, database_name: str): + rows = await self.get_internal_database().execute( + """ + SELECT + key, + value + FROM datasette_metadata_database_entries + WHERE database_name = ? + """, + [database_name], + ) + return dict(rows) + + async def get_resource_metadata(self, database_name: str, resource_name: str): + rows = await self.get_internal_database().execute( + """ + SELECT + key, + value + FROM datasette_metadata_resource_entries + WHERE database_name = ? + AND resource_name = ? + """, + [database_name, resource_name], + ) + return dict(rows) - for hook_dbs in pm.hook.get_metadata( - datasette=self, key=key, database=database, table=table - ): - metadata = self._metadata_recursive_update(metadata, hook_dbs) - - # security precaution!! don't allow anything in the local config - # to be overwritten. this is a temporary measure, not sure if this - # is a good idea long term or maybe if it should just be a concern - # of the plugin's implemtnation - metadata = self._metadata_recursive_update(metadata, self._metadata_local) - - databases = metadata.get("databases") or {} - - search_list = [] - if database is not None: - search_list.append(databases.get(database) or {}) - if table is not None: - table_metadata = ((databases.get(database) or {}).get("tables") or {}).get( - table - ) or {} - search_list.insert(0, table_metadata) - - search_list.append(metadata) - if not fallback: - # No fallback allowed, so just use the first one in the list - search_list = search_list[:1] - if key is not None: - for item in search_list: - if key in item: - return item[key] - return None - else: - # Return the merged list - m = {} - for item in search_list: - m.update(item) - return m + async def get_column_metadata( + self, database_name: str, resource_name: str, column_name: str + ): + rows = await self.get_internal_database().execute( + """ + SELECT + key, + value + FROM datasette_metadata_column_entries + WHERE database_name = ? + AND resource_name = ? + AND column_name = ? + """, + [database_name, resource_name, column_name], + ) + return dict(rows) + + async def set_instance_metadata(self, key: str, value: str): + # TODO upsert only supported on SQLite 3.24.0 (2018-06-04) + await self.get_internal_database().execute_write( + """ + INSERT INTO datasette_metadata_instance_entries(key, value) + VALUES(?, ?) + ON CONFLICT(key) DO UPDATE SET value = excluded.value; + """, + [key, value], + ) - @property - def _metadata(self): - return self.metadata() + async def set_database_metadata(self, database_name: str, key: str, value: str): + # TODO upsert only supported on SQLite 3.24.0 (2018-06-04) + await self.get_internal_database().execute_write( + """ + INSERT INTO datasette_metadata_database_entries(database_name, key, value) + VALUES(?, ?, ?) + ON CONFLICT(database_name, key) DO UPDATE SET value = excluded.value; + """, + [database_name, key, value], + ) + + async def set_resource_metadata( + self, database_name: str, resource_name: str, key: str, value: str + ): + # TODO upsert only supported on SQLite 3.24.0 (2018-06-04) + await self.get_internal_database().execute_write( + """ + INSERT INTO datasette_metadata_resource_entries(database_name, resource_name, key, value) + VALUES(?, ?, ?, ?) + ON CONFLICT(database_name, resource_name, key) DO UPDATE SET value = excluded.value; + """, + [database_name, resource_name, key, value], + ) + + async def set_column_metadata( + self, + database_name: str, + resource_name: str, + column_name: str, + key: str, + value: str, + ): + # TODO upsert only supported on SQLite 3.24.0 (2018-06-04) + await self.get_internal_database().execute_write( + """ + INSERT INTO datasette_metadata_column_entries(database_name, resource_name, column_name, key, value) + VALUES(?, ?, ?, ?, ?) + ON CONFLICT(database_name, resource_name, column_name, key) DO UPDATE SET value = excluded.value; + """, + [database_name, resource_name, column_name, key, value], + ) def get_internal_database(self): return self._internal_database @@ -774,20 +862,6 @@ async def get_canned_query(self, database_name, query_name, actor): if query: return query - def update_with_inherited_metadata(self, metadata): - # Fills in source/license with defaults, if available - metadata.update( - { - "source": metadata.get("source") or self.metadata("source"), - "source_url": metadata.get("source_url") or self.metadata("source_url"), - "license": metadata.get("license") or self.metadata("license"), - "license_url": metadata.get("license_url") - or self.metadata("license_url"), - "about": metadata.get("about") or self.metadata("about"), - "about_url": metadata.get("about_url") or self.metadata("about_url"), - } - ) - def _prepare_connection(self, conn, database): conn.row_factory = sqlite3.Row conn.text_factory = lambda x: str(x, "utf-8", "replace") @@ -1079,11 +1153,6 @@ def absolute_url(self, request, path): url = "https://" + url[len("http://") :] return url - def _register_custom_units(self): - """Register any custom units defined in the metadata.json with Pint""" - for unit in self.metadata("custom_units") or []: - ureg.define(unit) - def _connected_databases(self): return [ { @@ -1436,10 +1505,6 @@ def add_route(view, regex): ), r"/:memory:(?P.*)$", ) - add_route( - JsonDataView.as_view(self, "metadata.json", lambda: self.metadata()), - r"/-/metadata(\.(?Pjson))?$", - ) add_route( JsonDataView.as_view(self, "versions.json", self._versions), r"/-/versions(\.(?Pjson))?$", @@ -1585,7 +1650,6 @@ async def resolve_row(self, request): def app(self): """Returns an ASGI app function that serves the whole of Datasette""" routes = self._routes() - self._register_custom_units() async def setup_db(): # First time server starts up, calculate table counts for immutable databases diff --git a/datasette/default_menu_links.py b/datasette/default_menu_links.py index 56f481ef50..22e6e46abd 100644 --- a/datasette/default_menu_links.py +++ b/datasette/default_menu_links.py @@ -17,10 +17,6 @@ async def inner(): "href": datasette.urls.path("/-/versions"), "label": "Version info", }, - { - "href": datasette.urls.path("/-/metadata"), - "label": "Metadata", - }, { "href": datasette.urls.path("/-/settings"), "label": "Settings", diff --git a/datasette/facets.py b/datasette/facets.py index f1cfc68f02..ccd854610c 100644 --- a/datasette/facets.py +++ b/datasette/facets.py @@ -103,10 +103,15 @@ def get_facet_size(self): max_returned_rows = self.ds.setting("max_returned_rows") table_facet_size = None if self.table: - tables_metadata = self.ds.metadata("tables", database=self.database) or {} - table_metadata = tables_metadata.get(self.table) or {} - if table_metadata: - table_facet_size = table_metadata.get("facet_size") + config_facet_size = ( + self.ds.config.get("databases", {}) + .get(self.database, {}) + .get("tables", {}) + .get(self.table, {}) + .get("facet_size") + ) + if config_facet_size: + table_facet_size = config_facet_size custom_facet_size = self.request.args.get("_facet_size") if custom_facet_size: if custom_facet_size == "max": diff --git a/datasette/hookspecs.py b/datasette/hookspecs.py index 35468cc3ea..bcc2e22901 100644 --- a/datasette/hookspecs.py +++ b/datasette/hookspecs.py @@ -10,11 +10,6 @@ def startup(datasette): """Fires directly after Datasette first starts running""" -@hookspec -def get_metadata(datasette, key, database, table): - """Return metadata to be merged into Datasette's metadata dictionary""" - - @hookspec def asgi_wrapper(datasette): """Returns an ASGI middleware callable to wrap our ASGI application with""" diff --git a/datasette/renderer.py b/datasette/renderer.py index a446e69d0e..483c81e97a 100644 --- a/datasette/renderer.py +++ b/datasette/renderer.py @@ -56,7 +56,6 @@ def json_renderer(request, args, data, error, truncated=None): if truncated is not None: data["truncated"] = truncated - if shape == "arrayfirst": if not data["rows"]: data = [] diff --git a/datasette/utils/internal_db.py b/datasette/utils/internal_db.py index dbfcceb47d..6a5e08cba5 100644 --- a/datasette/utils/internal_db.py +++ b/datasette/utils/internal_db.py @@ -63,6 +63,43 @@ async def init_internal_db(db): """ ).strip() await db.execute_write_script(create_tables_sql) + await initialize_metadata_tables(db) + + +async def initialize_metadata_tables(db): + await db.execute_write_script( + """ + CREATE TABLE IF NOT EXISTS datasette_metadata_instance_entries( + key text, + value text, + unique(key) + ); + + CREATE TABLE IF NOT EXISTS datasette_metadata_database_entries( + database_name text, + key text, + value text, + unique(database_name, key) + ); + + CREATE TABLE IF NOT EXISTS datasette_metadata_resource_entries( + database_name text, + resource_name text, + key text, + value text, + unique(database_name, resource_name, key) + ); + + CREATE TABLE IF NOT EXISTS datasette_metadata_column_entries( + database_name text, + resource_name text, + column_name text, + key text, + value text, + unique(database_name, resource_name, column_name, key) + ); + """ + ) async def populate_schema_tables(internal_db, db): diff --git a/datasette/views/base.py b/datasette/views/base.py index 9d7a854c39..2e78b0a5a1 100644 --- a/datasette/views/base.py +++ b/datasette/views/base.py @@ -274,10 +274,6 @@ async def get(self, request): end = time.perf_counter() data["query_ms"] = (end - start) * 1000 - for key in ("source", "source_url", "license", "license_url"): - value = self.ds.metadata(key) - if value: - data[key] = value # Special case for .jsono extension - redirect to _shape=objects if _format == "jsono": @@ -385,7 +381,7 @@ async def get(self, request): }, } if "metadata" not in context: - context["metadata"] = self.ds.metadata() + context["metadata"] = await self.ds.get_instance_metadata() r = await self.render(templates, request=request, context=context) if status_code is not None: r.status = status_code diff --git a/datasette/views/database.py b/datasette/views/database.py index 851ae21fa1..2698a0ebaa 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -63,8 +63,7 @@ async def get(self, request, datasette): if format_ not in ("html", "json"): raise NotFound("Invalid format: {}".format(format_)) - metadata = (datasette.metadata("databases") or {}).get(database, {}) - datasette.update_with_inherited_metadata(metadata) + metadata = await datasette.get_database_metadata(database) sql_views = [] for view_name in await db.view_names(): @@ -131,6 +130,7 @@ async def database_actions(): "table_columns": ( await _table_columns(datasette, database) if allow_execute_sql else {} ), + "metadata": await datasette.get_database_metadata(database), } if format_ == "json": @@ -625,8 +625,7 @@ async def fetch_data_for_csv(request, _next=None): ) } ) - metadata = (datasette.metadata("databases") or {}).get(database, {}) - datasette.update_with_inherited_metadata(metadata) + metadata = await datasette.get_database_metadata(database) renderers = {} for key, (_, can_render) in datasette.renderers.items(): diff --git a/datasette/views/index.py b/datasette/views/index.py index 6546b7ae83..a3178f5331 100644 --- a/datasette/views/index.py +++ b/datasette/views/index.py @@ -132,7 +132,13 @@ async def get(self, request): if self.ds.cors: add_cors_headers(headers) return Response( - json.dumps({db["name"]: db for db in databases}, cls=CustomJSONEncoder), + json.dumps( + { + "databases": {db["name"]: db for db in databases}, + "metadata": await self.ds.get_instance_metadata(), + }, + cls=CustomJSONEncoder, + ), content_type="application/json; charset=utf-8", headers=headers, ) @@ -151,7 +157,7 @@ async def get(self, request): request=request, context={ "databases": databases, - "metadata": self.ds.metadata(), + "metadata": await self.ds.get_instance_metadata(), "datasette_version": __version__, "private": not await self.ds.permission_allowed( None, "view-instance" diff --git a/datasette/views/row.py b/datasette/views/row.py index 49d390f6be..6180446f1b 100644 --- a/datasette/views/row.py +++ b/datasette/views/row.py @@ -85,10 +85,6 @@ async def template_data(): "_table.html", ], "row_actions": row_actions, - "metadata": (self.ds.metadata("databases") or {}) - .get(database, {}) - .get("tables", {}) - .get(table, {}), "top_row": make_slot_function( "top_row", self.ds, @@ -97,6 +93,7 @@ async def template_data(): table=resolved.table, row=rows[0], ), + "metadata": {}, } data = { diff --git a/datasette/views/table.py b/datasette/views/table.py index ba03241dbc..e3bfb26060 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -147,7 +147,21 @@ async def display_columns_and_rows( """Returns columns, rows for specified table - including fancy foreign key treatment""" sortable_columns = sortable_columns or set() db = datasette.databases[database_name] - column_descriptions = datasette.metadata("columns", database_name, table_name) or {} + column_descriptions = dict( + await datasette.get_internal_database().execute( + """ + SELECT + column_name, + value + FROM datasette_metadata_column_entries + WHERE database_name = ? + AND resource_name = ? + AND key = 'description' + """, + [database_name, table_name], + ) + ) + column_details = { col.name: col for col in await db.table_column_details(table_name) } @@ -1478,14 +1492,22 @@ async def extra_query(): async def extra_metadata(): "Metadata about the table and database" - metadata = ( - (datasette.metadata("databases") or {}) - .get(database_name, {}) - .get("tables", {}) - .get(table_name, {}) + tablemetadata = await datasette.get_resource_metadata(database_name, table_name) + + rows = await datasette.get_internal_database().execute( + """ + SELECT + column_name, + value + FROM datasette_metadata_column_entries + WHERE database_name = ? + AND resource_name = ? + AND key = 'description' + """, + [database_name, table_name], ) - datasette.update_with_inherited_metadata(metadata) - return metadata + tablemetadata["columns"] = dict(rows) + return tablemetadata async def extra_database(): return database_name diff --git a/docs/codespell-ignore-words.txt b/docs/codespell-ignore-words.txt index 1959863f60..072f32b46d 100644 --- a/docs/codespell-ignore-words.txt +++ b/docs/codespell-ignore-words.txt @@ -2,4 +2,5 @@ alls fo ro te -ths \ No newline at end of file +ths +notin \ No newline at end of file diff --git a/docs/plugin_hooks.rst b/docs/plugin_hooks.rst index 972f3856b8..f4ef5d64d5 100644 --- a/docs/plugin_hooks.rst +++ b/docs/plugin_hooks.rst @@ -2002,6 +2002,7 @@ This example logs events to a `datasette_events` table in a database called `eve from datasette import hookimpl import json + @hookimpl def startup(datasette): async def inner(): @@ -2031,7 +2032,11 @@ This example logs events to a `datasette_events` table in a database called `eve insert into datasette_events (event_type, created, actor, properties) values (?, strftime('%Y-%m-%d %H:%M:%S', 'now'), ?, ?) """, - (event.name, json.dumps(event.actor), json.dumps(properties)), + ( + event.name, + json.dumps(event.actor), + json.dumps(properties), + ), ) return inner diff --git a/tests/test_api.py b/tests/test_api.py index 4ad55d7253..bd7346776c 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -29,8 +29,19 @@ async def test_homepage(ds_client): assert response.status_code == 200 assert "application/json; charset=utf-8" == response.headers["content-type"] data = response.json() - assert data.keys() == {"fixtures": 0}.keys() - d = data["fixtures"] + assert sorted(list(data.get("metadata").keys())) == [ + "about", + "about_url", + "description_html", + "license", + "license_url", + "source", + "source_url", + "title", + ] + databases = data.get("databases") + assert databases.keys() == {"fixtures": 0}.keys() + d = databases["fixtures"] assert d["name"] == "fixtures" assert isinstance(d["tables_count"], int) assert isinstance(len(d["tables_and_views_truncated"]), int) @@ -45,7 +56,8 @@ async def test_homepage_sort_by_relationships(ds_client): response = await ds_client.get("/.json?_sort=relationships") assert response.status_code == 200 tables = [ - t["name"] for t in response.json()["fixtures"]["tables_and_views_truncated"] + t["name"] + for t in response.json()["databases"]["fixtures"]["tables_and_views_truncated"] ] assert tables == [ "simple_primary_key", @@ -590,21 +602,24 @@ def test_no_files_uses_memory_database(app_client_no_files): response = app_client_no_files.get("/.json") assert response.status == 200 assert { - "_memory": { - "name": "_memory", - "hash": None, - "color": "a6c7b9", - "path": "/_memory", - "tables_and_views_truncated": [], - "tables_and_views_more": False, - "tables_count": 0, - "table_rows_sum": 0, - "show_table_row_counts": False, - "hidden_table_rows_sum": 0, - "hidden_tables_count": 0, - "views_count": 0, - "private": False, - } + "databases": { + "_memory": { + "name": "_memory", + "hash": None, + "color": "a6c7b9", + "path": "/_memory", + "tables_and_views_truncated": [], + "tables_and_views_more": False, + "tables_count": 0, + "table_rows_sum": 0, + "show_table_row_counts": False, + "hidden_table_rows_sum": 0, + "hidden_tables_count": 0, + "views_count": 0, + "private": False, + }, + }, + "metadata": {}, } == response.json # Try that SQL query response = app_client_no_files.get( @@ -768,12 +783,6 @@ def test_databases_json(app_client_two_attached_databases_one_immutable): assert False == fixtures_database["is_memory"] -@pytest.mark.asyncio -async def test_metadata_json(ds_client): - response = await ds_client.get("/-/metadata.json") - assert response.json() == ds_client.ds.metadata() - - @pytest.mark.asyncio async def test_threads_json(ds_client): response = await ds_client.get("/-/threads.json") @@ -1039,8 +1048,8 @@ async def test_tilde_encoded_database_names(db_name): ds = Datasette() ds.add_memory_database(db_name) response = await ds.client.get("/.json") - assert db_name in response.json().keys() - path = response.json()[db_name]["path"] + assert db_name in response.json()["databases"].keys() + path = response.json()["databases"][db_name]["path"] # And the JSON for that database response2 = await ds.client.get(path + ".json") assert response2.status_code == 200 @@ -1083,6 +1092,7 @@ async def test_config_json(config, expected): @pytest.mark.asyncio +@pytest.mark.skip(reason="rm?") @pytest.mark.parametrize( "metadata,expected_config,expected_metadata", ( diff --git a/tests/test_cli.py b/tests/test_cli.py index bda17eed00..486852cd4b 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -159,8 +159,8 @@ def test_metadata_yaml(): internal=None, ) client = _TestClient(ds) - response = client.get("/-/metadata.json") - assert {"title": "Hello from YAML"} == response.json + response = client.get("/.json") + assert {"title": "Hello from YAML"} == response.json["metadata"] @mock.patch("datasette.cli.run_module") diff --git a/tests/test_config_dir.py b/tests/test_config_dir.py index 66114a27db..46a6d341b4 100644 --- a/tests/test_config_dir.py +++ b/tests/test_config_dir.py @@ -99,12 +99,6 @@ def config_dir_client(config_dir): yield _TestClient(ds) -def test_metadata(config_dir_client): - response = config_dir_client.get("/-/metadata.json") - assert 200 == response.status - assert METADATA == response.json - - def test_settings(config_dir_client): response = config_dir_client.get("/-/settings.json") assert 200 == response.status @@ -149,17 +143,6 @@ def test_databases(config_dir_client): assert db["is_mutable"] == (expected_name != "immutable") -@pytest.mark.parametrize("filename", ("metadata.yml", "metadata.yaml")) -def test_metadata_yaml(tmp_path_factory, filename): - config_dir = tmp_path_factory.mktemp("yaml-config-dir") - (config_dir / filename).write_text("title: Title from metadata", "utf-8") - ds = Datasette([], config_dir=config_dir) - client = _TestClient(ds) - response = client.get("/-/metadata.json") - assert 200 == response.status - assert {"title": "Title from metadata"} == response.json - - def test_store_config_dir(config_dir_client): ds = config_dir_client.ds diff --git a/tests/test_facets.py b/tests/test_facets.py index 7634410864..023efcf092 100644 --- a/tests/test_facets.py +++ b/tests/test_facets.py @@ -584,9 +584,9 @@ async def test_facet_size(): data5 = response5.json() assert len(data5["facet_results"]["results"]["city"]["results"]) == 20 # Now try messing with facet_size in the table metadata - orig_metadata = ds._metadata_local + orig_config = ds.config try: - ds._metadata_local = { + ds.config = { "databases": { "test_facet_size": {"tables": {"neighbourhoods": {"facet_size": 6}}} } @@ -597,7 +597,7 @@ async def test_facet_size(): data6 = response6.json() assert len(data6["facet_results"]["results"]["city"]["results"]) == 6 # Setting it to max bumps it up to 50 again - ds._metadata_local["databases"]["test_facet_size"]["tables"]["neighbourhoods"][ + ds.config["databases"]["test_facet_size"]["tables"]["neighbourhoods"][ "facet_size" ] = "max" data7 = ( @@ -605,7 +605,7 @@ async def test_facet_size(): ).json() assert len(data7["facet_results"]["results"]["city"]["results"]) == 20 finally: - ds._metadata_local = orig_metadata + ds.config = orig_config def test_other_types_of_facet_in_metadata(): @@ -655,7 +655,6 @@ async def test_facet_against_in_memory_database(): to_insert = [{"name": "one", "name2": "1"} for _ in range(800)] + [ {"name": "two", "name2": "2"} for _ in range(300) ] - print(to_insert) await db.execute_write_many( "insert into t (name, name2) values (:name, :name2)", to_insert ) diff --git a/tests/test_html.py b/tests/test_html.py index 42b290c86e..de732d2c47 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -446,7 +446,7 @@ async def test_database_metadata(ds_client): soup.find("div", {"class": "metadata-description"}) ) # The source/license should be inherited - assert_footer_links(soup) + # assert_footer_links(soup) TODO(alex) ensure @pytest.mark.asyncio @@ -459,7 +459,7 @@ async def test_database_metadata_with_custom_sql(ds_client): # Description should be custom assert "Custom SQL query returning" in soup.find("h3").text # The source/license should be inherited - assert_footer_links(soup) + # assert_footer_links(soup)TODO(alex) ensure def test_database_download_for_immutable(): @@ -752,14 +752,6 @@ async def test_blob_download_invalid_messages(ds_client, path, expected_message) assert expected_message in response.text -@pytest.mark.asyncio -async def test_metadata_json_html(ds_client): - response = await ds_client.get("/-/metadata") - assert response.status_code == 200 - pre = Soup(response.content, "html.parser").find("pre") - assert ds_client.ds.metadata() == json.loads(pre.text) - - @pytest.mark.asyncio @pytest.mark.parametrize( "path", @@ -931,7 +923,7 @@ def test_edit_sql_link_not_shown_if_user_lacks_permission(permission_allowed): [ (None, None, None), ("test", None, ["/-/permissions"]), - ("root", ["/-/permissions", "/-/allow-debug", "/-/metadata"], None), + ("root", ["/-/permissions", "/-/allow-debug"], None), ], ) async def test_navigation_menu_links( diff --git a/tests/test_permissions.py b/tests/test_permissions.py index 7279531911..fe91ec3de8 100644 --- a/tests/test_permissions.py +++ b/tests/test_permissions.py @@ -453,7 +453,6 @@ def view_instance_client(): "/", "/fixtures", "/fixtures/facetable", - "/-/metadata", "/-/versions", "/-/plugins", "/-/settings", diff --git a/tests/test_plugins.py b/tests/test_plugins.py index 9c8d02ecc4..cc2e16c83c 100644 --- a/tests/test_plugins.py +++ b/tests/test_plugins.py @@ -331,14 +331,14 @@ def test_hook_extra_template_vars(restore_working_directory): with make_app_client( template_dir=str(pathlib.Path(__file__).parent / "test_templates") ) as client: - response = client.get("/-/metadata") + response = client.get("/-/versions") assert response.status_code == 200 extra_template_vars = json.loads( Soup(response.text, "html.parser").select("pre.extra_template_vars")[0].text ) assert { "template": "show_json.html", - "scope_path": "/-/metadata", + "scope_path": "/-/versions", "columns": None, } == extra_template_vars extra_template_vars_from_awaitable = json.loads( @@ -349,7 +349,7 @@ def test_hook_extra_template_vars(restore_working_directory): assert { "template": "show_json.html", "awaitable": True, - "scope_path": "/-/metadata", + "scope_path": "/-/versions", } == extra_template_vars_from_awaitable @@ -357,7 +357,7 @@ def test_plugins_async_template_function(restore_working_directory): with make_app_client( template_dir=str(pathlib.Path(__file__).parent / "test_templates") ) as client: - response = client.get("/-/metadata") + response = client.get("/-/versions") assert response.status_code == 200 extra_from_awaitable_function = ( Soup(response.text, "html.parser") @@ -422,7 +422,7 @@ def extra_template_vars(view_name): ("/fixtures", "database"), ("/fixtures/units", "table"), ("/fixtures/units/1", "row"), - ("/-/metadata", "json_data"), + ("/-/versions", "json_data"), ("/fixtures?sql=select+1", "database"), ), ) @@ -1073,36 +1073,6 @@ def test_hook_skip_csrf(app_client): assert second_missing_csrf_response.status_code == 403 -@pytest.mark.asyncio -async def test_hook_get_metadata(ds_client): - try: - orig_metadata = ds_client.ds._metadata_local - ds_client.ds._metadata_local = { - "title": "Testing get_metadata hook!", - "databases": {"from-local": {"title": "Hello from local metadata"}}, - } - og_pm_hook_get_metadata = pm.hook.get_metadata - - def get_metadata_mock(*args, **kwargs): - return [ - { - "databases": { - "from-hook": {"title": "Hello from the plugin hook"}, - "from-local": {"title": "This will be overwritten!"}, - } - } - ] - - pm.hook.get_metadata = get_metadata_mock - meta = ds_client.ds.metadata() - assert "Testing get_metadata hook!" == meta["title"] - assert "Hello from local metadata" == meta["databases"]["from-local"]["title"] - assert "Hello from the plugin hook" == meta["databases"]["from-hook"]["title"] - pm.hook.get_metadata = og_pm_hook_get_metadata - finally: - ds_client.ds._metadata_local = orig_metadata - - def _extract_commands(output): lines = output.split("Commands:\n", 1)[1].split("\n") return {line.split()[0].replace("*", "") for line in lines if line.strip()} @@ -1550,6 +1520,7 @@ async def test_hook_register_events(): assert any(k.__name__ == "OneEvent" for k in datasette.event_classes) +@pytest.mark.skip(reason="TODO") @pytest.mark.parametrize( "metadata,config,expected_metadata,expected_config", ( diff --git a/tests/test_routes.py b/tests/test_routes.py index 85945decf0..b2d317598a 100644 --- a/tests/test_routes.py +++ b/tests/test_routes.py @@ -43,8 +43,6 @@ def routes(): "RowView", {"format": "json", "database": "foo", "pks": "1", "table": "humbug"}, ), - ("/-/metadata.json", "JsonDataView", {"format": "json"}), - ("/-/metadata", "JsonDataView", {"format": None}), ), ) def test_routes(routes, path, expected_name, expected_matches): diff --git a/tests/test_table_html.py b/tests/test_table_html.py index 2a6586636c..fcf914a6d9 100644 --- a/tests/test_table_html.py +++ b/tests/test_table_html.py @@ -792,8 +792,6 @@ async def test_table_metadata(ds_client): assert "Simple primary key" == inner_html( soup.find("div", {"class": "metadata-description"}) ) - # The source/license should be inherited - assert_footer_links(soup) @pytest.mark.asyncio @@ -1101,8 +1099,8 @@ async def test_column_metadata(ds_client): soup = Soup(response.text, "html.parser") dl = soup.find("dl") assert [(dt.text, dt.nextSibling.text) for dt in dl.findAll("dt")] == [ - ("name", "The name of the attraction"), ("address", "The street address for the attraction"), + ("name", "The name of the attraction"), ] assert ( soup.select("th[data-column=name]")[0]["data-column-description"] From c698d008e08396ca0d288febbb9f656d96db83a9 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 11 Jun 2024 10:03:57 -0700 Subject: [PATCH 03/48] Only test first wheel, fixes surprise bug https://github.com/simonw/datasette/issues/2351#issuecomment-2161211173 --- test-in-pyodide-with-shot-scraper.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-in-pyodide-with-shot-scraper.sh b/test-in-pyodide-with-shot-scraper.sh index 0c140818f4..acd9346595 100755 --- a/test-in-pyodide-with-shot-scraper.sh +++ b/test-in-pyodide-with-shot-scraper.sh @@ -6,7 +6,7 @@ set -e python3 -m build # Find name of wheel, strip off the dist/ -wheel=$(basename $(ls dist/*.whl)) +wheel=$(basename $(ls dist/*.whl) | head -n 1) # Create a blank index page echo ' From 9a3c3bfcc7d30d65514421bd315f1b8b5b735b86 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 11 Jun 2024 10:11:34 -0700 Subject: [PATCH 04/48] Fix for pyodide test failure, refs #2351 --- test-in-pyodide-with-shot-scraper.sh | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/test-in-pyodide-with-shot-scraper.sh b/test-in-pyodide-with-shot-scraper.sh index acd9346595..5845a0c3fd 100755 --- a/test-in-pyodide-with-shot-scraper.sh +++ b/test-in-pyodide-with-shot-scraper.sh @@ -18,6 +18,13 @@ cd dist python3 -m http.server 8529 & cd .. +# Register the kill_server function to be called on script exit +kill_server() { + pkill -f 'http.server 8529' +} +trap kill_server EXIT + + shot-scraper javascript http://localhost:8529/ " async () => { let pyodide = await loadPyodide(); @@ -26,6 +33,8 @@ async () => { import micropip await micropip.install('h11==0.12.0') await micropip.install('httpx==0.23') + # To avoid 'from typing_extensions import deprecated' error: + await micropip.install('typing-extensions>=4.12.2') await micropip.install('http://localhost:8529/$wheel') import ssl import setuptools @@ -38,7 +47,4 @@ async () => { } return 'Test passed!'; } -" - -# Shut down the server -pkill -f 'http.server 8529' +" \ No newline at end of file From 7437d40e5dd4d614bb769e16c0c1b96c6c19647f Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 11 Jun 2024 10:17:02 -0700 Subject: [PATCH 05/48] , closes #2348 --- datasette/templates/base.html | 2 +- datasette/templates/patterns.html | 2 +- tests/test_html.py | 2 ++ 3 files changed, 4 insertions(+), 2 deletions(-) diff --git a/datasette/templates/base.html b/datasette/templates/base.html index afb8dcd3da..51f602b66e 100644 --- a/datasette/templates/base.html +++ b/datasette/templates/base.html @@ -1,5 +1,5 @@ {% import "_crumbs.html" as crumbs with context %} - + {% block title %}{% endblock %} diff --git a/datasette/templates/patterns.html b/datasette/templates/patterns.html index cb0daf9a8e..24fe55314e 100644 --- a/datasette/templates/patterns.html +++ b/datasette/templates/patterns.html @@ -1,5 +1,5 @@ - + Datasette: Pattern Portfolio diff --git a/tests/test_html.py b/tests/test_html.py index de732d2c47..a83f0e47db 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -21,6 +21,8 @@ def test_homepage(app_client_two_attached_databases): response = app_client_two_attached_databases.get("/") assert response.status_code == 200 assert "text/html; charset=utf-8" == response.headers["content-type"] + # Should have a html lang="en" attribute + assert '' in response.text soup = Soup(response.content, "html.parser") assert "Datasette Fixtures" == soup.find("h1").text assert ( From 2b6bfddafc1f3ad95bbb334e34a026964db4a813 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Tue, 11 Jun 2024 14:04:55 -0700 Subject: [PATCH 06/48] Workaround for #2353 --- datasette/utils/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index e110891119..8075420278 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -1146,7 +1146,7 @@ async def derive_named_parameters(db: "Database", sql: str) -> List[str]: try: results = await db.execute(explain, {p: None for p in possible_params}) return [row["p4"].lstrip(":") for row in results if row["opcode"] == "Variable"] - except sqlite3.DatabaseError: + except (sqlite3.DatabaseError, AttributeError): return possible_params From 780deaa275849a39a283ec7b57a0351aac18a047 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 12 Jun 2024 16:12:05 -0700 Subject: [PATCH 07/48] Reminder about how to deploy a release branch --- docs/contributing.rst | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/contributing.rst b/docs/contributing.rst index b678e6379d..45330a8347 100644 --- a/docs/contributing.rst +++ b/docs/contributing.rst @@ -284,6 +284,10 @@ You can generate the list of issue references for a specific release by copying To create the tag for the release, create `a new release `__ on GitHub matching the new version number. You can convert the release notes to Markdown by copying and pasting the rendered HTML into this `Paste to Markdown tool `__. +Don't forget to create the release from the correct branch - usually ``main``, but sometimes ``0.64.x`` or similar for a bugfix release. + +While the release is running you can confirm that the correct commits made it into the release using the https://github.com/simonw/datasette/compare/0.64.6...0.64.7 URL. + Finally, post a news item about the release on `datasette.io `__ by editing the `news.yaml `__ file in that site's repository. .. _contributing_alpha_beta: From b39b01a89066b1322bb9f731529e636d707e19f6 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 12 Jun 2024 16:21:07 -0700 Subject: [PATCH 08/48] Copy across release notes from 0.64.7 Refs #2353 --- docs/changelog.rst | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index ebed499fef..90bf75e1d6 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,13 @@ Changelog ========= +.. _v0_64_7: + +0.64.7 (2023-06-12) +------------------- + +- Fixed a bug where canned queries with named parameters threw an error when run against SQLite 3.46.0. (:issue:`2353`) + .. _v1_0_a13: 1.0a13 (2024-03-12) From d118d5c5bbb675f35baa5c376ee297b510dccfc0 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 12 Jun 2024 16:51:07 -0700 Subject: [PATCH 09/48] named_parameters(sql) sync function, refs #2354 Also refs #2353 and #2352 --- datasette/utils/__init__.py | 33 ++++++++++++++++++++++++--------- datasette/views/database.py | 6 ++---- docs/internals.rst | 10 +++++----- tests/test_utils.py | 8 ++++++-- 4 files changed, 37 insertions(+), 20 deletions(-) diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 8075420278..b3b51c5fa7 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -1131,23 +1131,38 @@ class StartupError(Exception): pass -_re_named_parameter = re.compile(":([a-zA-Z0-9_]+)") +_single_line_comment_re = re.compile(r"--.*") +_multi_line_comment_re = re.compile(r"/\*.*?\*/", re.DOTALL) +_single_quote_re = re.compile(r"'(?:''|[^'])*'") +_double_quote_re = re.compile(r'"(?:\"\"|[^"])*"') +_named_param_re = re.compile(r":(\w+)") @documented -async def derive_named_parameters(db: "Database", sql: str) -> List[str]: +def named_parameters(sql: str) -> List[str]: """ Given a SQL statement, return a list of named parameters that are used in the statement e.g. for ``select * from foo where id=:id`` this would return ``["id"]`` """ - explain = "explain {}".format(sql.strip().rstrip(";")) - possible_params = _re_named_parameter.findall(sql) - try: - results = await db.execute(explain, {p: None for p in possible_params}) - return [row["p4"].lstrip(":") for row in results if row["opcode"] == "Variable"] - except (sqlite3.DatabaseError, AttributeError): - return possible_params + # Remove single-line comments + sql = _single_line_comment_re.sub("", sql) + # Remove multi-line comments + sql = _multi_line_comment_re.sub("", sql) + # Remove single-quoted strings + sql = _single_quote_re.sub("", sql) + # Remove double-quoted strings + sql = _double_quote_re.sub("", sql) + # Extract parameters from what is left + return _named_param_re.findall(sql) + + +async def derive_named_parameters(db: "Database", sql: str) -> List[str]: + """ + This undocumented but stable method exists for backwards compatibility + with plugins that were using it before it switched to named_parameters() + """ + return named_parameters(sql) def add_cors_headers(headers): diff --git a/datasette/views/database.py b/datasette/views/database.py index 2698a0ebaa..1d76c5e047 100644 --- a/datasette/views/database.py +++ b/datasette/views/database.py @@ -17,7 +17,7 @@ add_cors_headers, await_me_maybe, call_with_supported_arguments, - derive_named_parameters, + named_parameters as derive_named_parameters, format_bytes, make_slot_function, tilde_decode, @@ -484,9 +484,7 @@ async def get(self, request, datasette): if canned_query and canned_query.get("params"): named_parameters = canned_query["params"] if not named_parameters: - named_parameters = await derive_named_parameters( - datasette.get_database(database), sql - ) + named_parameters = derive_named_parameters(sql) named_parameter_values = { named_parameter: params.get(named_parameter) or "" for named_parameter in named_parameters diff --git a/docs/internals.rst b/docs/internals.rst index 795856594d..38e66a57b7 100644 --- a/docs/internals.rst +++ b/docs/internals.rst @@ -1256,14 +1256,14 @@ Utility function for calling ``await`` on a return value if it is awaitable, oth .. autofunction:: datasette.utils.await_me_maybe -.. _internals_utils_derive_named_parameters: +.. _internals_utils_named_parameters: -derive_named_parameters(db, sql) --------------------------------- +named_parameters(sql) +--------------------- -Derive the list of named parameters referenced in a SQL query, using an ``explain`` query executed against the provided database. +Derive the list of ``:named`` parameters referenced in a SQL query. -.. autofunction:: datasette.utils.derive_named_parameters +.. autofunction:: datasette.utils.named_parameters .. _internals_tilde_encoding: diff --git a/tests/test_utils.py b/tests/test_utils.py index 254b130055..88a4532ad1 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -612,10 +612,14 @@ def test_parse_metadata(content, expected): ("select this is invalid :one, :two, :three", ["one", "two", "three"]), ), ) -async def test_derive_named_parameters(sql, expected): +@pytest.mark.parametrize("use_async_version", (False, True)) +async def test_named_parameters(sql, expected, use_async_version): ds = Datasette([], memory=True) db = ds.get_database("_memory") - params = await utils.derive_named_parameters(db, sql) + if use_async_version: + params = await utils.derive_named_parameters(db, sql) + else: + params = utils.named_parameters(sql) assert params == expected From 64a125b860eda6f821ce5a67f48b13fd173d8671 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Wed, 12 Jun 2024 16:56:46 -0700 Subject: [PATCH 10/48] Removed unnecessary comments, refs #2354 --- datasette/utils/__init__.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index b3b51c5fa7..c87de5f003 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -1145,13 +1145,9 @@ def named_parameters(sql: str) -> List[str]: e.g. for ``select * from foo where id=:id`` this would return ``["id"]`` """ - # Remove single-line comments sql = _single_line_comment_re.sub("", sql) - # Remove multi-line comments sql = _multi_line_comment_re.sub("", sql) - # Remove single-quoted strings sql = _single_quote_re.sub("", sql) - # Remove double-quoted strings sql = _double_quote_re.sub("", sql) # Extract parameters from what is left return _named_param_re.findall(sql) From 8f86d2af6a5b90bd0bf12812b4f37c0fbfc1bceb Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Thu, 13 Jun 2024 10:09:45 -0700 Subject: [PATCH 11/48] Test against multiple SQLite versions (#2352) * Use sqlite-versions action for testing multiple versions --- .github/workflows/test-sqlite-support.yml | 53 +++++++++++++++++++++++ tests/test_csv.py | 1 + 2 files changed, 54 insertions(+) create mode 100644 .github/workflows/test-sqlite-support.yml diff --git a/.github/workflows/test-sqlite-support.yml b/.github/workflows/test-sqlite-support.yml new file mode 100644 index 0000000000..7882e05d54 --- /dev/null +++ b/.github/workflows/test-sqlite-support.yml @@ -0,0 +1,53 @@ +name: Test SQLite versions + +on: [push, pull_request] + +permissions: + contents: read + +jobs: + test: + runs-on: ${{ matrix.platform }} + continue-on-error: true + strategy: + matrix: + platform: [ubuntu-latest] + python-version: [ "3.8", "3.9", "3.10", "3.11", "3.12"] + sqlite-version: [ + #"3", # latest version + "3.46", + #"3.45", + #"3.27", + #"3.26", + "3.25", + #"3.25.3", # 2018-09-25, window functions breaks test_upsert for some reason on 3.10, skip for now + #"3.24", # 2018-06-04, added UPSERT support + #"3.23.1" # 2018-04-10, before UPSERT + ] + steps: + - uses: actions/checkout@v4 + - name: Set up Python ${{ matrix.python-version }} + uses: actions/setup-python@v5 + with: + python-version: ${{ matrix.python-version }} + allow-prereleases: true + cache: pip + cache-dependency-path: setup.py + - name: Set up SQLite ${{ matrix.sqlite-version }} + uses: asg017/sqlite-versions@71ea0de37ae739c33e447af91ba71dda8fcf22e6 + with: + version: ${{ matrix.sqlite-version }} + cflags: "-DSQLITE_ENABLE_DESERIALIZE -DSQLITE_ENABLE_FTS5 -DSQLITE_ENABLE_FTS4 -DSQLITE_ENABLE_FTS3_PARENTHESIS -DSQLITE_ENABLE_RTREE -DSQLITE_ENABLE_JSON1" + - run: python3 -c "import sqlite3; print(sqlite3.sqlite_version)" + - run: echo $LD_LIBRARY_PATH + - name: Build extension for --load-extension test + run: |- + (cd tests && gcc ext.c -fPIC -shared -o ext.so) + - name: Install dependencies + run: | + pip install -e '.[test]' + pip freeze + - name: Run tests + run: | + pytest -n auto -m "not serial" + pytest -m "serial" diff --git a/tests/test_csv.py b/tests/test_csv.py index 9f772f8985..d4c072c050 100644 --- a/tests/test_csv.py +++ b/tests/test_csv.py @@ -189,6 +189,7 @@ async def test_csv_with_non_ascii_characters(ds_client): assert response.text == "text,number\r\nšœš¢š­š¢šžš¬,1\r\nbob,2\r\n" +@pytest.mark.skip(reason="flakey") def test_max_csv_mb(app_client_csv_max_mb_one): # This query deliberately generates a really long string # should be 100*100*100*2 = roughly 2MB From 45c27603d2b1f3536eccee7bdf20b9626a128bb3 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 13 Jun 2024 10:15:38 -0700 Subject: [PATCH 12/48] xfail two flaky tests, #2355, #2356 --- tests/test_api_write.py | 1 + tests/test_csv.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 6a7ddeb61d..2b57a99a0f 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -1359,6 +1359,7 @@ async def test_create_table_permissions( @pytest.mark.asyncio +@pytest.mark.xfail(reason="Flaky, see https://github.com/simonw/datasette/issues/2356") @pytest.mark.parametrize( "input,expected_rows_after", ( diff --git a/tests/test_csv.py b/tests/test_csv.py index d4c072c050..6dbe693b64 100644 --- a/tests/test_csv.py +++ b/tests/test_csv.py @@ -189,7 +189,7 @@ async def test_csv_with_non_ascii_characters(ds_client): assert response.text == "text,number\r\nšœš¢š­š¢šžš¬,1\r\nbob,2\r\n" -@pytest.mark.skip(reason="flakey") +@pytest.mark.xfail(reason="Flaky, see https://github.com/simonw/datasette/issues/2355") def test_max_csv_mb(app_client_csv_max_mb_one): # This query deliberately generates a really long string # should be 100*100*100*2 = roughly 2MB From 93534fd3d0c34aac9ff5f8677f1a8d8bddf886f9 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Thu, 13 Jun 2024 10:19:26 -0700 Subject: [PATCH 13/48] Show response.text on test_upsert failure, refs #2356 --- tests/test_api_write.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 2b57a99a0f..7dbe12b555 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -537,7 +537,7 @@ async def test_upsert(ds_write, initial, input, expected_rows, should_return): json=input, headers=_headers(token), ) - assert response.status_code == 200 + assert response.status_code == 200, response.text assert response.json()["ok"] is True # Analytics event From 62686114eee429d6f5e6badc4f11076b35c987df Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 21 Jun 2024 16:02:15 -0700 Subject: [PATCH 14/48] Do not show database name in Database Not Found error, refs #2359 --- datasette/app.py | 4 +--- datasette/utils/asgi.py | 4 ++-- tests/test_api_write.py | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 8020c5dac2..f732f95c0a 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1617,9 +1617,7 @@ async def resolve_database(self, request): try: return self.get_database(route=database_route) except KeyError: - raise DatabaseNotFound( - "Database not found: {}".format(database_route), database_route - ) + raise DatabaseNotFound(database_route) async def resolve_table(self, request): db = await self.resolve_database(request) diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index 2fad1d425b..b3f00bb3c5 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -23,9 +23,9 @@ class NotFound(Base400): class DatabaseNotFound(NotFound): - def __init__(self, message, database_name): - super().__init__(message) + def __init__(self, database_name): self.database_name = database_name + super().__init__("Database not found") class TableNotFound(NotFound): diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 7dbe12b555..2cd8785808 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -133,7 +133,7 @@ async def test_insert_rows(ds_write, return_rows): {}, None, 404, - ["Database not found: data2"], + ["Database not found"], ), ( "/data/docs2/-/insert", From 7316dd4ac629d3b0e93a92b07cd6e565e2a66a07 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 21 Jun 2024 16:09:20 -0700 Subject: [PATCH 15/48] Fix for TableNotFound, refs #2359 --- datasette/app.py | 4 +--- datasette/utils/asgi.py | 4 ++-- tests/test_api_write.py | 10 +++++----- tests/test_table_api.py | 2 +- tests/test_table_html.py | 2 +- 5 files changed, 10 insertions(+), 12 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index f732f95c0a..001cf4c366 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1628,9 +1628,7 @@ async def resolve_table(self, request): if not table_exists: is_view = await db.view_exists(table_name) if not (table_exists or is_view): - raise TableNotFound( - "Table not found: {}".format(table_name), db.name, table_name - ) + raise TableNotFound(db.name, table_name) return ResolvedTable(db, table_name, is_view) async def resolve_row(self, request): diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index b3f00bb3c5..6bdba714e4 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -29,8 +29,8 @@ def __init__(self, database_name): class TableNotFound(NotFound): - def __init__(self, message, database_name, table): - super().__init__(message) + def __init__(self, database_name, table): + super().__init__("Table not found") self.database_name = database_name self.table = table diff --git a/tests/test_api_write.py b/tests/test_api_write.py index 2cd8785808..94eb902b2b 100644 --- a/tests/test_api_write.py +++ b/tests/test_api_write.py @@ -140,7 +140,7 @@ async def test_insert_rows(ds_write, return_rows): {}, None, 404, - ["Table not found: docs2"], + ["Table not found"], ), ( "/data/docs/-/insert", @@ -274,7 +274,7 @@ async def test_insert_rows(ds_write, return_rows): {"rows": [{"title": "Test"}]}, None, 404, - ["Table not found: badtable"], + ["Table not found"], ), # missing primary key ( @@ -598,7 +598,7 @@ async def test_delete_row_errors(ds_write, scenario): assert ( response.json()["errors"] == ["Permission denied"] if scenario == "no_token" - else ["Table not found: bad_table"] + else ["Table not found"] ) assert len((await ds_write.client.get("/data/docs.json?_shape=array")).json()) == 1 @@ -703,7 +703,7 @@ async def test_update_row_check_permission(ds_write, scenario): assert ( response.json()["errors"] == ["Permission denied"] if scenario == "no_token" - else ["Table not found: bad_table"] + else ["Table not found"] ) @@ -830,7 +830,7 @@ async def test_drop_table(ds_write, scenario): assert response.json()["ok"] is False expected_error = "Permission denied" if scenario == "bad_table": - expected_error = "Table not found: bad_table" + expected_error = "Table not found" elif scenario == "immutable": expected_error = "Database is immutable" assert response.json()["errors"] == [expected_error] diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 5893095045..8360157d7b 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -36,7 +36,7 @@ async def test_table_json(ds_client): async def test_table_not_exists_json(ds_client): assert (await ds_client.get("/fixtures/blah.json")).json() == { "ok": False, - "error": "Table not found: blah", + "error": "Table not found", "status": 404, "title": None, } diff --git a/tests/test_table_html.py b/tests/test_table_html.py index fcf914a6d9..1a9ed5404c 100644 --- a/tests/test_table_html.py +++ b/tests/test_table_html.py @@ -535,7 +535,7 @@ async def test_csv_json_export_links_include_labels_if_foreign_keys(ds_client): @pytest.mark.asyncio async def test_table_not_exists(ds_client): - assert "Table not found: blah" in (await ds_client.get("/fixtures/blah")).text + assert "Table not found" in (await ds_client.get("/fixtures/blah")).text @pytest.mark.asyncio From 263788906aad8d53cb29369983f72a3b861bb4da Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 21 Jun 2024 16:10:16 -0700 Subject: [PATCH 16/48] Fix for RowNotFound, refs #2359 --- datasette/app.py | 4 +--- datasette/utils/asgi.py | 4 ++-- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/datasette/app.py b/datasette/app.py index 001cf4c366..1c5d6a3727 100644 --- a/datasette/app.py +++ b/datasette/app.py @@ -1638,9 +1638,7 @@ async def resolve_row(self, request): results = await db.execute(sql, params, truncate=True) row = results.first() if row is None: - raise RowNotFound( - "Row not found: {}".format(pk_values), db.name, table_name, pk_values - ) + raise RowNotFound(db.name, table_name, pk_values) return ResolvedRow(db, table_name, sql, params, pks, pk_values, results.first()) def app(self): diff --git a/datasette/utils/asgi.py b/datasette/utils/asgi.py index 6bdba714e4..1699847e16 100644 --- a/datasette/utils/asgi.py +++ b/datasette/utils/asgi.py @@ -36,8 +36,8 @@ def __init__(self, database_name, table): class RowNotFound(NotFound): - def __init__(self, message, database_name, table, pk_values): - super().__init__(message) + def __init__(self, database_name, table, pk_values): + super().__init__("Row not found") self.database_name = database_name self.table_name = table self.pk_values = pk_values From c2e8e5085b5ccc49121feaf11b57eb7d5af53bd9 Mon Sep 17 00:00:00 2001 From: Simon Willison Date: Fri, 21 Jun 2024 16:36:58 -0700 Subject: [PATCH 17/48] Release notes for 0.64.8 on main --- docs/changelog.rst | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/docs/changelog.rst b/docs/changelog.rst index 90bf75e1d6..dab17bc64a 100644 --- a/docs/changelog.rst +++ b/docs/changelog.rst @@ -4,6 +4,14 @@ Changelog ========= +.. _v0_64_8: + +0.64.8 (2023-06-21) +------------------- + +- Security improvement: 404 pages used to reflect content from the URL path, which could be used to display misleading information to Datasette users. 404 errors no longer display additional information from the URL. (:issue:`2359`) +- Backported a better fix for correctly extracting named parameters from canned query SQL against SQLite 3.46.0. (:issue:`2353`) + .. _v0_64_7: 0.64.7 (2023-06-12) From 56adfff8d2112c5890888af4da65d842ba7f6f86 Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Tue, 2 Jul 2024 09:45:15 -0700 Subject: [PATCH 18/48] Bump the python-packages group across 1 directory with 4 updates (#2362) Bumps the python-packages group with 4 updates in the / directory: [sphinx](https://github.com/sphinx-doc/sphinx), [furo](https://github.com/pradyunsg/furo), [blacken-docs](https://github.com/adamchainz/blacken-docs) and [black](https://github.com/psf/black). Updates `sphinx` from 7.2.6 to 7.3.7 - [Release notes](https://github.com/sphinx-doc/sphinx/releases) - [Changelog](https://github.com/sphinx-doc/sphinx/blob/master/CHANGES.rst) - [Commits](https://github.com/sphinx-doc/sphinx/compare/v7.2.6...v7.3.7) Updates `furo` from 2024.1.29 to 2024.5.6 - [Release notes](https://github.com/pradyunsg/furo/releases) - [Changelog](https://github.com/pradyunsg/furo/blob/main/docs/changelog.md) - [Commits](https://github.com/pradyunsg/furo/compare/2024.01.29...2024.05.06) Updates `blacken-docs` from 1.16.0 to 1.18.0 - [Changelog](https://github.com/adamchainz/blacken-docs/blob/main/CHANGELOG.rst) - [Commits](https://github.com/adamchainz/blacken-docs/compare/1.16.0...1.18.0) Updates `black` from 24.2.0 to 24.4.2 - [Release notes](https://github.com/psf/black/releases) - [Changelog](https://github.com/psf/black/blob/main/CHANGES.md) - [Commits](https://github.com/psf/black/compare/24.2.0...24.4.2) --- updated-dependencies: - dependency-name: sphinx dependency-type: direct:development update-type: version-update:semver-minor dependency-group: python-packages - dependency-name: furo dependency-type: direct:development update-type: version-update:semver-minor dependency-group: python-packages - dependency-name: blacken-docs dependency-type: direct:development update-type: version-update:semver-minor dependency-group: python-packages - dependency-name: black dependency-type: direct:development update-type: version-update:semver-minor dependency-group: python-packages ... Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> --- setup.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/setup.py b/setup.py index e51d824743..fa61729ff0 100644 --- a/setup.py +++ b/setup.py @@ -70,8 +70,8 @@ def get_version(): """, extras_require={ "docs": [ - "Sphinx==7.2.6", - "furo==2024.1.29", + "Sphinx==7.3.7", + "furo==2024.5.6", "sphinx-autobuild", "codespell>=2.2.5", "blacken-docs", @@ -84,8 +84,8 @@ def get_version(): "pytest-xdist>=2.2.1", "pytest-asyncio>=0.17", "beautifulsoup4>=4.8.1", - "black==24.2.0", - "blacken-docs==1.16.0", + "black==24.4.2", + "blacken-docs==1.18.0", "pytest-timeout>=1.4.2", "trustme>=0.7", "cogapp>=3.3.0", From a23c2aee006bea5f50c13e687dac77c66df1888b Mon Sep 17 00:00:00 2001 From: Alex Garcia Date: Mon, 15 Jul 2024 10:33:51 -0700 Subject: [PATCH 19/48] 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