Use dash-encoding for row PKs and ?_next=, refs #1439

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]
 

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:

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),

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": []},

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",
+            [
+                [
+                    '<td class="col-pk1 type-str">a</td>',
+                    '<td class="col-pk2 type-str">b</td>',
+                    '<td class="col-content type-str">c</td>',
+                ]
+            ],
+        ),
+        (
+            "/fixtures/compound_primary_key/a-2Fb,-2Ec-2Dd",
+            [
+                [
+                    '<td class="col-pk1 type-str">a/b</td>',
+                    '<td class="col-pk2 type-str">.c-d</td>',
+                    '<td class="col-content type-str">c</td>',
+                ]
+            ],
+        ),
+    ),
+)
+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 = [
-        [
-            '<td class="col-pk1 type-str">a</td>',
-            '<td class="col-pk2 type-str">b</td>',
-            '<td class="col-content type-str">c</td>',
-        ]
-    ]
     assert expected == [
         [str(td) for td in tr.select("td")] for tr in table.select("tbody tr")
     ]

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]

tests/test_table_html.py

@@ -563,11 +563,17 @@ def test_table_html_compound_primary_key(app_client):
             '<td class="col-pk1 type-str">a</td>',
             '<td class="col-pk2 type-str">b</td>',
             '<td class="col-content type-str">c</td>',
-        ]
+        ],
+        [
+            '<td class="col-Link type-pk"><a href="/fixtures/compound_primary_key/a-2Fb,-2Ec-2Dd">a/b,.c-d</a></td>',
+            '<td class="col-pk1 type-str">a/b</td>',
+            '<td class="col-pk2 type-str">.c-d</td>',
+            '<td class="col-content type-str">c</td>',
+        ],
     ]
-    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):

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(