diff --git a/datasette/utils/__init__.py b/datasette/utils/__init__.py index 19c81d3b..aef85fbb 100644 --- a/datasette/utils/__init__.py +++ b/datasette/utils/__init__.py @@ -111,12 +111,12 @@ async def await_me_maybe(value: typing.Any) -> typing.Any: def urlsafe_components(token): - """Splits token on commas and URL decodes each component""" - return [urllib.parse.unquote_plus(b) for b in token.split(",")] + """Splits token on commas and dash-decodes each component""" + return [dash_decode(b) for b in token.split(",")] def path_from_row_pks(row, pks, use_rowid, quote=True): - """Generate an optionally URL-quoted unique identifier + """Generate an optionally dash-quoted unique identifier for a row from its primary keys.""" if use_rowid: bits = [row["rowid"]] @@ -125,7 +125,7 @@ def path_from_row_pks(row, pks, use_rowid, quote=True): row[pk]["value"] if isinstance(row[pk], dict) else row[pk] for pk in pks ] if quote: - bits = [urllib.parse.quote_plus(str(bit)) for bit in bits] + bits = [dash_encode(str(bit)) for bit in bits] else: bits = [str(bit) for bit in bits] diff --git a/datasette/views/table.py b/datasette/views/table.py index be9e9c3b..8ccb7371 100644 --- a/datasette/views/table.py +++ b/datasette/views/table.py @@ -12,6 +12,7 @@ from datasette.utils import ( MultiParams, append_querystring, compound_keys_after_sql, + dash_encode, escape_sqlite, filters_should_redirect, is_url, @@ -765,7 +766,7 @@ class TableView(RowTableShared): if prefix is None: prefix = "$null" else: - prefix = urllib.parse.quote_plus(str(prefix)) + prefix = dash_encode(str(prefix)) next_value = f"{prefix},{next_value}" added_args = {"_next": next_value} if sort: diff --git a/tests/fixtures.py b/tests/fixtures.py index 26f0cf7b..11f09c41 100644 --- a/tests/fixtures.py +++ b/tests/fixtures.py @@ -406,6 +406,7 @@ CREATE TABLE compound_primary_key ( ); INSERT INTO compound_primary_key VALUES ('a', 'b', 'c'); +INSERT INTO compound_primary_key VALUES ('a/b', '.c-d', 'c'); CREATE TABLE compound_three_primary_keys ( pk1 varchar(30), diff --git a/tests/test_api.py b/tests/test_api.py index 57471af2..caedd1ac 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -143,7 +143,7 @@ def test_database_page(app_client): "name": "compound_primary_key", "columns": ["pk1", "pk2", "content"], "primary_keys": ["pk1", "pk2"], - "count": 1, + "count": 2, "hidden": False, "fts_table": None, "foreign_keys": {"incoming": [], "outgoing": []}, diff --git a/tests/test_html.py b/tests/test_html.py index d5f4250d..a7ad4248 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -345,20 +345,38 @@ def test_row_links_from_other_tables(app_client, path, expected_text, expected_l assert link == expected_link -def test_row_html_compound_primary_key(app_client): - response = app_client.get("/fixtures/compound_primary_key/a,b") +@pytest.mark.parametrize( + "path,expected", + ( + ( + "/fixtures/compound_primary_key/a,b", + [ + [ + 'a', + 'b', + 'c', + ] + ], + ), + ( + "/fixtures/compound_primary_key/a-2Fb,-2Ec-2Dd", + [ + [ + 'a/b', + '.c-d', + 'c', + ] + ], + ), + ), +) +def test_row_html_compound_primary_key(app_client, path, expected): + response = app_client.get(path) assert response.status == 200 table = Soup(response.body, "html.parser").find("table") assert ["pk1", "pk2", "content"] == [ th.string.strip() for th in table.select("thead th") ] - expected = [ - [ - 'a', - 'b', - 'c', - ] - ] assert expected == [ [str(td) for td in tr.select("td")] for tr in table.select("tbody tr") ] diff --git a/tests/test_table_api.py b/tests/test_table_api.py index 6a6daed5..cc38d392 100644 --- a/tests/test_table_api.py +++ b/tests/test_table_api.py @@ -136,7 +136,10 @@ def test_table_shape_object(app_client): def test_table_shape_object_compound_primary_key(app_client): response = app_client.get("/fixtures/compound_primary_key.json?_shape=object") - assert {"a,b": {"pk1": "a", "pk2": "b", "content": "c"}} == response.json + assert response.json == { + "a,b": {"pk1": "a", "pk2": "b", "content": "c"}, + "a-2Fb,-2Ec-2Dd": {"pk1": "a/b", "pk2": ".c-d", "content": "c"}, + } def test_table_with_slashes_in_name(app_client): @@ -308,7 +311,7 @@ def test_sortable(app_client, query_string, sort_key, human_description_en): path = response.json["next_url"] if path: path = path.replace("http://localhost", "") - assert 5 == page + assert page == 5 expected = list(generate_sortable_rows(201)) expected.sort(key=sort_key) assert [r["content"] for r in expected] == [r["content"] for r in fetched] diff --git a/tests/test_table_html.py b/tests/test_table_html.py index 021268c3..77d97d80 100644 --- a/tests/test_table_html.py +++ b/tests/test_table_html.py @@ -563,11 +563,17 @@ def test_table_html_compound_primary_key(app_client): 'a', 'b', 'c', - ] + ], + [ + 'a/b,.c-d', + 'a/b', + '.c-d', + 'c', + ], ] - assert expected == [ + assert [ [str(td) for td in tr.select("td")] for tr in table.select("tbody tr") - ] + ] == expected def test_table_html_foreign_key_links(app_client): diff --git a/tests/test_utils.py b/tests/test_utils.py index 3d5dee38..1c3ab495 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -93,7 +93,7 @@ def test_path_with_replaced_args(path, args, expected): "row,pks,expected_path", [ ({"A": "foo", "B": "bar"}, ["A", "B"], "foo,bar"), - ({"A": "f,o", "B": "bar"}, ["A", "B"], "f%2Co,bar"), + ({"A": "f,o", "B": "bar"}, ["A", "B"], "f-2Co,bar"), ({"A": 123}, ["A"], "123"), ( utils.CustomRow(