mirror of
https://github.com/simonw/datasette.git
synced 2026-07-09 17:14:35 +02:00
Unify page-size parameters on _size with table semantics
The stored query lists silently clamped out-of-range ?_size= values (a request for 5000 quietly returned 1000) and did not accept the max keyword. They now share the table view semantics via a new parse_size_limit() helper: blank means default, "max" means the maximum (max_returned_rows for query lists), negative or non-integer values are a 400, and values over the maximum are a 400 instead of being silently clamped. The /-/allowed and /-/rules debug endpoints renamed their bare page/page_size parameters to _page/_size, matching the underscore grammar used by every other system parameter, with the same validation (400 instead of silently capping page_size at 200). Their HTML debug pages and next_url/previous_url builders use the new names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GrHZSypDfMnym1tM5XJAFZ
This commit is contained in:
parent
404ee4c3a7
commit
3a0ea58557
13 changed files with 137 additions and 63 deletions
|
|
@ -49,7 +49,7 @@
|
|||
|
||||
<div class="form-section">
|
||||
<label for="page_size">Page size:</label>
|
||||
<input type="number" id="page_size" name="page_size" value="50" min="1" max="200" style="max-width: 100px;">
|
||||
<input type="number" id="page_size" name="_size" value="50" min="1" max="200" style="max-width: 100px;">
|
||||
<small>Number of results per page (max 200)</small>
|
||||
</div>
|
||||
|
||||
|
|
@ -88,7 +88,7 @@ const hasDebugPermission = {{ 'true' if has_debug_permission else 'false' }};
|
|||
(function() {
|
||||
const params = populateFormFromURL();
|
||||
const action = params.get('action');
|
||||
const page = params.get('page');
|
||||
const page = params.get('_page');
|
||||
if (action) {
|
||||
fetchResults(page ? parseInt(page) : 1);
|
||||
}
|
||||
|
|
@ -102,14 +102,14 @@ async function fetchResults(page = 1) {
|
|||
const params = new URLSearchParams();
|
||||
|
||||
for (const [key, value] of formData.entries()) {
|
||||
if (value && key !== 'page_size') {
|
||||
if (value && key !== '_size' && key !== '_page') {
|
||||
params.append(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
const pageSize = document.getElementById('page_size').value || '50';
|
||||
params.append('page', page.toString());
|
||||
params.append('page_size', pageSize);
|
||||
params.append('_page', page.toString());
|
||||
params.append('_size', pageSize);
|
||||
|
||||
try {
|
||||
const response = await fetch('{{ urls.path("-/allowed.json") }}?' + params.toString(), {
|
||||
|
|
|
|||
|
|
@ -37,7 +37,7 @@
|
|||
|
||||
<div class="form-section">
|
||||
<label for="page_size">Page size:</label>
|
||||
<input type="number" id="page_size" name="page_size" value="50" min="1" max="200" style="max-width: 100px;">
|
||||
<input type="number" id="page_size" name="_size" value="50" min="1" max="200" style="max-width: 100px;">
|
||||
<small>Number of results per page (max 200)</small>
|
||||
</div>
|
||||
|
||||
|
|
@ -75,7 +75,7 @@ const submitBtn = document.getElementById('submit-btn');
|
|||
(function() {
|
||||
const params = populateFormFromURL();
|
||||
const action = params.get('action');
|
||||
const page = params.get('page');
|
||||
const page = params.get('_page');
|
||||
if (action) {
|
||||
fetchResults(page ? parseInt(page) : 1);
|
||||
}
|
||||
|
|
@ -89,14 +89,14 @@ async function fetchResults(page = 1) {
|
|||
const params = new URLSearchParams();
|
||||
|
||||
for (const [key, value] of formData.entries()) {
|
||||
if (value && key !== 'page_size') {
|
||||
if (value && key !== '_size' && key !== '_page') {
|
||||
params.append(key, value);
|
||||
}
|
||||
}
|
||||
|
||||
const pageSize = document.getElementById('page_size').value || '50';
|
||||
params.append('page', page.toString());
|
||||
params.append('page_size', pageSize);
|
||||
params.append('_page', page.toString());
|
||||
params.append('_size', pageSize);
|
||||
|
||||
try {
|
||||
const response = await fetch('{{ urls.path("-/rules.json") }}?' + params.toString(), {
|
||||
|
|
|
|||
|
|
@ -1294,6 +1294,28 @@ async def derive_named_parameters(db: "Database", sql: str) -> List[str]:
|
|||
return named_parameters(sql)
|
||||
|
||||
|
||||
def parse_size_limit(value, default, maximum, name="_size"):
|
||||
"""
|
||||
Parse a page-size parameter using the same semantics as the table
|
||||
view's ?_size=: blank means default, "max" means maximum, integers
|
||||
must be 0 or greater and no larger than maximum. Raises ValueError
|
||||
with a message suitable for a 400 response.
|
||||
"""
|
||||
if value in (None, ""):
|
||||
return default
|
||||
if value == "max":
|
||||
return maximum
|
||||
try:
|
||||
size = int(value)
|
||||
if size < 0:
|
||||
raise ValueError
|
||||
except ValueError:
|
||||
raise ValueError("{} must be a positive integer".format(name))
|
||||
if size > maximum:
|
||||
raise ValueError("{} must be <= {}".format(name, maximum))
|
||||
return size
|
||||
|
||||
|
||||
UNSTABLE_API_MESSAGE = (
|
||||
"This API is not part of Datasette's stable interface and may change at any time"
|
||||
)
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ from datasette.write_sql import (
|
|||
operation_is_write,
|
||||
)
|
||||
from datasette.utils import (
|
||||
parse_size_limit,
|
||||
named_parameters as derive_named_parameters,
|
||||
escape_sqlite,
|
||||
path_from_row_pks,
|
||||
|
|
@ -94,13 +95,11 @@ def _as_optional_bool(value, name):
|
|||
raise QueryValidationError("{} must be 0 or 1".format(name))
|
||||
|
||||
|
||||
def _query_list_limit(value, default=50):
|
||||
if value in (None, ""):
|
||||
return default
|
||||
def _query_list_limit(value, default, maximum):
|
||||
try:
|
||||
return min(max(1, int(value)), 1000)
|
||||
return parse_size_limit(value, default, maximum)
|
||||
except ValueError as ex:
|
||||
raise QueryValidationError("_size must be an integer") from ex
|
||||
raise QueryValidationError(str(ex)) from ex
|
||||
|
||||
|
||||
def _derived_query_parameters(sql):
|
||||
|
|
|
|||
|
|
@ -8,6 +8,7 @@ from datasette.utils.asgi import Response, Forbidden
|
|||
from datasette.utils import (
|
||||
UNSTABLE_API_MESSAGE,
|
||||
actor_matches_allow,
|
||||
parse_size_limit,
|
||||
add_cors_headers,
|
||||
await_me_maybe,
|
||||
error_body,
|
||||
|
|
@ -373,17 +374,17 @@ class AllowedResourcesView(BaseView):
|
|||
)
|
||||
|
||||
try:
|
||||
page = int(request.args.get("page", "1"))
|
||||
page_size = int(request.args.get("page_size", "50"))
|
||||
page = int(request.args.get("_page", "1"))
|
||||
if page < 1:
|
||||
raise ValueError
|
||||
except ValueError:
|
||||
return error_body("page and page_size must be integers", 400), 400
|
||||
if page < 1:
|
||||
return error_body("page must be >= 1", 400), 400
|
||||
if page_size < 1:
|
||||
return error_body("page_size must be >= 1", 400), 400
|
||||
max_page_size = 200
|
||||
if page_size > max_page_size:
|
||||
page_size = max_page_size
|
||||
return error_body("_page must be a positive integer", 400), 400
|
||||
try:
|
||||
page_size = parse_size_limit(
|
||||
request.args.get("_size"), default=50, maximum=200
|
||||
)
|
||||
except ValueError as ex:
|
||||
return error_body(str(ex), 400), 400
|
||||
offset = (page - 1) * page_size
|
||||
|
||||
# Use the simplified allowed_resources method
|
||||
|
|
@ -448,12 +449,12 @@ class AllowedResourcesView(BaseView):
|
|||
def build_page_url(page_number):
|
||||
pairs = []
|
||||
for key in request.args:
|
||||
if key in {"page", "page_size"}:
|
||||
if key in {"_page", "_size"}:
|
||||
continue
|
||||
for value in request.args.getlist(key):
|
||||
pairs.append((key, value))
|
||||
pairs.append(("page", str(page_number)))
|
||||
pairs.append(("page_size", str(page_size)))
|
||||
pairs.append(("_page", str(page_number)))
|
||||
pairs.append(("_size", str(page_size)))
|
||||
query = urllib.parse.urlencode(pairs)
|
||||
return f"{request.path}?{query}"
|
||||
|
||||
|
|
@ -511,19 +512,19 @@ class PermissionRulesView(BaseView):
|
|||
actor = request.actor if isinstance(request.actor, dict) else None
|
||||
|
||||
try:
|
||||
page = int(request.args.get("page", "1"))
|
||||
page_size = int(request.args.get("page_size", "50"))
|
||||
page = int(request.args.get("_page", "1"))
|
||||
if page < 1:
|
||||
raise ValueError
|
||||
except ValueError:
|
||||
return Response.json(
|
||||
error_body("page and page_size must be integers", 400), status=400
|
||||
error_body("_page must be a positive integer", 400), status=400
|
||||
)
|
||||
if page < 1:
|
||||
return Response.json(error_body("page must be >= 1", 400), status=400)
|
||||
if page_size < 1:
|
||||
return Response.json(error_body("page_size must be >= 1", 400), status=400)
|
||||
max_page_size = 200
|
||||
if page_size > max_page_size:
|
||||
page_size = max_page_size
|
||||
try:
|
||||
page_size = parse_size_limit(
|
||||
request.args.get("_size"), default=50, maximum=200
|
||||
)
|
||||
except ValueError as ex:
|
||||
return Response.json(error_body(str(ex), 400), status=400)
|
||||
offset = (page - 1) * page_size
|
||||
|
||||
from datasette.utils.actions_sql import build_permission_rules_sql
|
||||
|
|
@ -574,12 +575,12 @@ class PermissionRulesView(BaseView):
|
|||
def build_page_url(page_number):
|
||||
pairs = []
|
||||
for key in request.args:
|
||||
if key in {"page", "page_size"}:
|
||||
if key in {"_page", "_size"}:
|
||||
continue
|
||||
for value in request.args.getlist(key):
|
||||
pairs.append((key, value))
|
||||
pairs.append(("page", str(page_number)))
|
||||
pairs.append(("page_size", str(page_size)))
|
||||
pairs.append(("_page", str(page_number)))
|
||||
pairs.append(("_size", str(page_size)))
|
||||
query = urllib.parse.urlencode(pairs)
|
||||
return f"{request.path}?{query}"
|
||||
|
||||
|
|
|
|||
|
|
@ -90,6 +90,7 @@ class QueryListView(BaseView):
|
|||
limit = _query_list_limit(
|
||||
request.args.get("_size"),
|
||||
default=20 if format_ == "html" else 50,
|
||||
maximum=self.ds.max_returned_rows,
|
||||
)
|
||||
is_write = _as_optional_bool(request.args.get("is_write"), "is_write")
|
||||
is_private = _as_optional_bool(request.args.get("is_private"), "is_private")
|
||||
|
|
|
|||
|
|
@ -1162,7 +1162,7 @@ The ``/-/allowed`` endpoint displays resources that the current actor can access
|
|||
|
||||
This endpoint provides an interactive HTML form interface. Add ``.json`` to the URL path (e.g. ``/-/allowed.json``) to get the raw JSON response instead.
|
||||
|
||||
Pass ``?action=view-table`` (or another action) to select the action. Optional ``parent=`` and ``child=`` query parameters can narrow the results to a specific database/table pair.
|
||||
Pass ``?action=view-table`` (or another action) to select the action. Optional ``parent=`` and ``child=`` query parameters can narrow the results to a specific database/table pair. Results are paginated: ``?_size=`` sets the page size (default 50, maximum 200, ``max`` for the maximum) and ``?_page=`` selects a page.
|
||||
|
||||
This endpoint is publicly accessible to help users understand their own permissions. The potentially sensitive ``reason`` field is only shown to users with the ``permissions-debug`` permission - it shows the plugins and explanatory reasons that were responsible for each decision.
|
||||
|
||||
|
|
@ -1175,7 +1175,7 @@ The ``/-/rules`` endpoint displays all permission rules (both allow and deny) fo
|
|||
|
||||
This endpoint provides an interactive HTML form interface. Add ``.json`` to the URL path (e.g. ``/-/rules.json?action=view-table``) to get the raw JSON response instead.
|
||||
|
||||
Pass ``?action=`` as a query parameter to specify which action to check.
|
||||
Pass ``?action=`` as a query parameter to specify which action to check. The ``?_size=`` and ``?_page=`` pagination parameters work the same as on ``/-/allowed``.
|
||||
|
||||
This endpoint requires the ``permissions-debug`` permission.
|
||||
|
||||
|
|
|
|||
|
|
@ -95,7 +95,7 @@ Use the :ref:`ExecuteWriteView` JSON API to execute writable SQL programmaticall
|
|||
Stored query browsers
|
||||
---------------------
|
||||
|
||||
The ``/-/queries`` page lists stored queries across every database visible to the current actor. The ``/database-name/-/queries`` page lists stored queries for a single database.
|
||||
The ``/-/queries`` page lists stored queries across every database visible to the current actor. The ``/database-name/-/queries`` page lists stored queries for a single database. The JSON versions accept ``?_size=`` (default 50, ``max`` for the :ref:`setting_max_returned_rows` limit) and a ``?_next=`` pagination token.
|
||||
|
||||
These pages support search, pagination and filters for read-only or writable queries and private or public queries. Adding a ``.json`` extension to either URL returns the same list as JSON.
|
||||
|
||||
|
|
|
|||
|
|
@ -377,8 +377,8 @@ path always renders the HTML form; `.json` returns JSON.
|
|||
resources. Items gain a `reason` field if the actor also holds
|
||||
`permissions-debug`.
|
||||
- **Parameters:** `action` (required; missing → 400 canonical error, unknown
|
||||
→ 404), `parent`, `child` (requires `parent`), `page` (default 1),
|
||||
`page_size` (default 50, silently capped at 200).
|
||||
→ 404), `parent`, `child` (requires `parent`), `_page` (default 1),
|
||||
`_size` (default 50, maximum 200, accepts `max`; out-of-range → 400).
|
||||
- **Response:** `{"action", "actor_id", "page", "page_size", "total",
|
||||
"items": [{"parent", "child", "resource"}]}` with optional `next_url` /
|
||||
`previous_url`.
|
||||
|
|
@ -1031,8 +1031,9 @@ databases (`database`/`database_color` are null, `show_database` true).
|
|||
|
||||
- **Permissions:** no single gate; results filtered per query by
|
||||
`view-query` (private queries appear only for their owner).
|
||||
- **Parameters:** `_size` (default 20 HTML / **50 JSON**, clamped 1–1000;
|
||||
non-integer → 400), `_next` (cursor), `q` (substring search over
|
||||
- **Parameters:** `_size` (default 20 HTML / **50 JSON**; accepts `max`;
|
||||
values over `max_returned_rows` or non-integers → 400, matching table
|
||||
`_size` semantics), `_next` (cursor), `q` (substring search over
|
||||
name/title/description/sql), `is_write` / `is_private` (booleans; invalid →
|
||||
400 `"is_write must be 0 or 1"`), `source`, `owner_id`.
|
||||
- **Response** — 200:
|
||||
|
|
|
|||
|
|
@ -203,9 +203,13 @@ number.
|
|||
> **Status:** `next_url` now accompanies `next` in the default table JSON
|
||||
> keys (previously it required `?_extra=next_url`), so every response with
|
||||
> a `next` token also carries the ready-to-follow URL. Pagination tokens
|
||||
> are deliberately left undocumented as to their internal structure. The
|
||||
> `_size`/`page_size` naming and `has_more`/`total` differences remain
|
||||
> open.
|
||||
> are deliberately left undocumented as to their internal structure.
|
||||
> `_size` is now the single page-size parameter with uniform table-style
|
||||
> semantics everywhere: query lists accept `max` and 400 on out-of-range
|
||||
> values (previously silently clamped), and the `/-/allowed` and
|
||||
> `/-/rules` debug endpoints renamed `page`/`page_size` to
|
||||
> `_page`/`_size` with the same validation (400 instead of silent
|
||||
> capping at 200). The `has_more`/`total` differences remain open.
|
||||
|
||||
| Endpoint | Mechanism | Token | Extras |
|
||||
|---|---|---|---|
|
||||
|
|
|
|||
|
|
@ -612,3 +612,53 @@ async def test_threads_requires_permissions_debug(ds_error_shape):
|
|||
allowed = await ds_error_shape.client.get("/-/threads.json", actor={"id": "root"})
|
||||
assert allowed.status_code == 200
|
||||
assert allowed.json()["ok"] is True
|
||||
|
||||
|
||||
# _size is the one page-size parameter, with uniform validation
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_list_size_supports_max_keyword(ds_client):
|
||||
response = await ds_client.get("/fixtures/-/queries.json?_size=max")
|
||||
assert response.status_code == 200
|
||||
# ds_client runs with max_returned_rows=100
|
||||
assert response.json()["limit"] == 100
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_list_size_rejects_out_of_range(ds_client):
|
||||
response = await ds_client.get("/fixtures/-/queries.json?_size=5000")
|
||||
data = assert_canonical_error(response, 400)
|
||||
assert data["errors"] == ["_size must be <= 100"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_query_list_size_rejects_non_integer(ds_client):
|
||||
response = await ds_client.get("/fixtures/-/queries.json?_size=bananas")
|
||||
data = assert_canonical_error(response, 400)
|
||||
assert data["errors"] == ["_size must be a positive integer"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize("endpoint", ("allowed", "rules"))
|
||||
async def test_debug_endpoints_use_size_and_page_parameters(ds_error_shape, endpoint):
|
||||
base = "/-/{}.json?action=view-instance".format(endpoint)
|
||||
ok = await ds_error_shape.client.get(
|
||||
base + "&_size=1&_page=1", actor={"id": "root"}
|
||||
)
|
||||
assert ok.status_code == 200
|
||||
assert ok.json()["page_size"] == 1
|
||||
|
||||
max_size = await ds_error_shape.client.get(
|
||||
base + "&_size=max", actor={"id": "root"}
|
||||
)
|
||||
assert max_size.status_code == 200
|
||||
assert max_size.json()["page_size"] == 200
|
||||
|
||||
too_big = await ds_error_shape.client.get(base + "&_size=500", actor={"id": "root"})
|
||||
data = assert_canonical_error(too_big, 400)
|
||||
assert data["errors"] == ["_size must be <= 200"]
|
||||
|
||||
bad_page = await ds_error_shape.client.get(base + "&_page=0", actor={"id": "root"})
|
||||
data = assert_canonical_error(bad_page, 400)
|
||||
assert data["errors"] == ["_page must be a positive integer"]
|
||||
|
|
|
|||
|
|
@ -1363,12 +1363,12 @@ async def test_permission_debug_tabs_with_query_string(ds_client):
|
|||
|
||||
# Test /-/allowed with query string
|
||||
response = await ds_client.get(
|
||||
"/-/allowed?action=view-table&page_size=50", actor=actor
|
||||
"/-/allowed?action=view-table&_size=50", actor=actor
|
||||
)
|
||||
assert response.status_code == 200
|
||||
# Check that Rules and Check tabs have the query string
|
||||
assert 'href="/-/rules?action=view-table&page_size=50"' in response.text
|
||||
assert 'href="/-/check?action=view-table&page_size=50"' in response.text
|
||||
assert 'href="/-/rules?action=view-table&_size=50"' in response.text
|
||||
assert 'href="/-/check?action=view-table&_size=50"' in response.text
|
||||
# Playground and Actions should not have query string
|
||||
assert 'href="/-/permissions"' in response.text
|
||||
assert 'href="/-/actions"' in response.text
|
||||
|
|
|
|||
|
|
@ -137,9 +137,7 @@ async def test_allowed_json_pagination():
|
|||
await ds.refresh_schemas()
|
||||
|
||||
# Test page 1
|
||||
response = await ds.client.get(
|
||||
"/-/allowed.json?action=view-table&page_size=10&page=1"
|
||||
)
|
||||
response = await ds.client.get("/-/allowed.json?action=view-table&_size=10&_page=1")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["page"] == 1
|
||||
|
|
@ -147,9 +145,7 @@ async def test_allowed_json_pagination():
|
|||
assert len(data["items"]) == 10
|
||||
|
||||
# Test page 2
|
||||
response = await ds.client.get(
|
||||
"/-/allowed.json?action=view-table&page_size=10&page=2"
|
||||
)
|
||||
response = await ds.client.get("/-/allowed.json?action=view-table&_size=10&_page=2")
|
||||
assert response.status_code == 200
|
||||
data = response.json()
|
||||
assert data["page"] == 2
|
||||
|
|
@ -157,10 +153,10 @@ async def test_allowed_json_pagination():
|
|||
|
||||
# Verify items are different between pages
|
||||
response1 = await ds.client.get(
|
||||
"/-/allowed.json?action=view-table&page_size=10&page=1"
|
||||
"/-/allowed.json?action=view-table&_size=10&_page=1"
|
||||
)
|
||||
response2 = await ds.client.get(
|
||||
"/-/allowed.json?action=view-table&page_size=10&page=2"
|
||||
"/-/allowed.json?action=view-table&_size=10&_page=2"
|
||||
)
|
||||
items1 = {(item["parent"], item["child"]) for item in response1.json()["items"]}
|
||||
items2 = {(item["parent"], item["child"]) for item in response2.json()["items"]}
|
||||
|
|
@ -323,7 +319,7 @@ async def test_rules_json_pagination():
|
|||
|
||||
# Test basic pagination structure - just verify it returns paginated results
|
||||
response = await ds.client.get(
|
||||
"/-/rules.json?action=view-table&page_size=2&page=1",
|
||||
"/-/rules.json?action=view-table&_size=2&_page=1",
|
||||
actor={"id": "root"},
|
||||
)
|
||||
assert response.status_code == 200
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue