fix: make partial search pagination lossless (Closes #807)
This commit is contained in:
1 parent
e5f37a866c
commit
10f7cc832a
7 files changed
+239
-32
No files matched your search
@@ -8,6 +8,8 @@ from tests.dashboard_bundle import dashboard_bundle_text
|
||||
def test_api_requests_resolve_inside_dashboard_subpath():
|
||||
html = dashboard_bundle_text()
|
||||
api_paths = re.findall(r"fetch\(['\"]([^'\"]*api/v1/[^'\"]*)['\"]", html)
|
||||
commands = (Path(__file__).parents[1] / "frontend" / "commands.js").read_text()
|
||||
api_paths += re.findall(r"return ['\"]([^'\"]*api/v1/[^'\"]*)['\"]", commands)
|
||||
|
||||
assert api_paths
|
||||
assert {
|
||||
|
||||
@@ -56,6 +56,7 @@ def test_command_script_resolves_inside_dashboard_subpath():
|
||||
def test_mobile_search_renders_touch_sized_type_and_status_scope_controls():
|
||||
html = dashboard_bundle_text()
|
||||
css = (FRONTEND / "dashboard.css").read_text()
|
||||
commands = COMMANDS.read_text()
|
||||
|
||||
assert '<fieldset class="cmd-search-scope"' in html
|
||||
assert '<legend>Filter search results</legend>' in html
|
||||
@@ -68,13 +69,14 @@ def test_mobile_search_renders_touch_sized_type_and_status_scope_controls():
|
||||
assert '#cmd-search-kind, #cmd-search-state { min-height:44px;' in css
|
||||
assert "commandSearch.setScope(scope)" in html
|
||||
assert "taskOverlayHistory.update({ scope })" in html
|
||||
assert "'&kind=' + encodeURIComponent(scope.kind)" in html
|
||||
assert "'&state=' + encodeURIComponent(scope.state)" in html
|
||||
assert "'&kind=' + encodeURIComponent(scope.kind)" in commands
|
||||
assert "'&state=' + encodeURIComponent(scope.state)" in commands
|
||||
|
||||
|
||||
def test_mobile_search_exposes_accessible_repository_scope_and_forwards_it():
|
||||
html = dashboard_bundle_text()
|
||||
css = (FRONTEND / "dashboard.css").read_text()
|
||||
commands = COMMANDS.read_text()
|
||||
|
||||
assert '<label for="cmd-search-repository">Repository</label>' in html
|
||||
assert '<input id="cmd-search-repository"' in html
|
||||
@@ -82,7 +84,7 @@ def test_mobile_search_exposes_accessible_repository_scope_and_forwards_it():
|
||||
assert '<datalist id="cmd-search-repositories">' in html
|
||||
assert "api/v1/repositories/search?q=" in html
|
||||
assert "repository:qs('#cmd-search-repository').value" in html
|
||||
assert "'&repository=' + encodeURIComponent(scope.repository)" in html
|
||||
assert "'&repository=' + encodeURIComponent(scope.repository)" in commands
|
||||
assert '#cmd-search-repository' in css
|
||||
|
||||
|
||||
@@ -246,6 +248,44 @@ if (!ready || ready.items.length !== 1 || ready.partial !== true) {{
|
||||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_remote_command_search_retries_partial_continuation_without_losing_results():
|
||||
script = f"""
|
||||
const filterCommands = require({json.dumps(str(COMMANDS))});
|
||||
(async () => {{
|
||||
const requests = [];
|
||||
const states = [];
|
||||
const controller = filterCommands.createGlobalSearchController({{
|
||||
delay: 0,
|
||||
search: (query, signal, page, scope, continuation) => {{
|
||||
requests.push({{page, continuation}});
|
||||
if (requests.length === 1) return Promise.resolve({{
|
||||
items:[{{kind:'pull',repository:'a/b',number:2}}], partial:true,
|
||||
has_more:true, next_page:2, continuation:{{issues:1,pulls:null}}, failed_streams:['issue']
|
||||
}});
|
||||
return Promise.resolve({{
|
||||
items:[{{kind:'issue',repository:'a/b',number:1}}], partial:false,
|
||||
has_more:false, next_page:2, continuation:{{issues:null,pulls:null}}, failed_streams:[]
|
||||
}});
|
||||
}},
|
||||
onState: state => states.push(state),
|
||||
}});
|
||||
controller.setQuery('mobile');
|
||||
await new Promise(resolve => setTimeout(resolve, 0));
|
||||
await new Promise(resolve => setTimeout(resolve, 0));
|
||||
await controller.loadMore();
|
||||
const ready = states.filter(state => state.status === 'ready').at(-1);
|
||||
if (JSON.stringify(requests[1].continuation) !== JSON.stringify({{issues:1,pulls:null}})) {{
|
||||
throw new Error('retry did not preserve failed stream page: ' + JSON.stringify(requests));
|
||||
}}
|
||||
if (ready.partial || ready.items.length !== 2 || ready.items[0].kind !== 'pull' || ready.items[1].kind !== 'issue') {{
|
||||
throw new Error('recovered results were not appended: ' + JSON.stringify(ready));
|
||||
}}
|
||||
}})().catch(error => {{ console.error(error); process.exit(1); }});
|
||||
"""
|
||||
|
||||
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||
|
||||
|
||||
def test_remote_command_search_appends_deduplicated_pages_and_ignores_stale_load_more():
|
||||
script = f"""
|
||||
const filterCommands = require({json.dumps(str(COMMANDS))});
|
||||
@@ -281,6 +321,10 @@ if (ready.query !== 'release' || ready.items.length !== 1 || ready.items[0].numb
|
||||
def test_command_selection_wraps_for_arrow_keys():
|
||||
script = f"""
|
||||
const commands = require({json.dumps(str(COMMANDS))});
|
||||
if (commands.searchUrl('mobile', 2, {{kind:'all',state:'open'}}, {{issues:1,pulls:3}}) !==
|
||||
'api/v1/search?q=mobile&limit=10&page=2&kind=all&state=open&issues_page=1&pulls_page=3') {{
|
||||
throw new Error('independent continuation query was not encoded');
|
||||
}}
|
||||
if (commands.nextSelection(-1, 'ArrowDown', 3) !== 0) throw new Error('should select first');
|
||||
if (commands.nextSelection(2, 'ArrowDown', 3) !== 0) throw new Error('should wrap down');
|
||||
if (commands.nextSelection(0, 'ArrowUp', 3) !== 2) throw new Error('should wrap up');
|
||||
@@ -296,10 +340,11 @@ def test_palette_exposes_accessible_global_work_search_under_dashboard_subpath()
|
||||
assert 'role="combobox"' in html
|
||||
assert 'aria-controls="cmd-results"' in html
|
||||
assert 'role="listbox"' in html
|
||||
assert "fetch('api/v1/search?q='" in html
|
||||
assert "fetch(filterCommands.searchUrl(" in html
|
||||
assert "async function searchGlobalWork(query, signal, page = 1, scope" in html
|
||||
assert "signal," in html
|
||||
assert "Some results are temporarily unavailable." in html
|
||||
assert "Retry missing results" in html
|
||||
assert 'id="cmd-load-more"' in html
|
||||
assert urljoin(
|
||||
"https://forge.alexanderwhitestone.com/dashboard/",
|
||||
|
||||
+114
-7
@@ -28,6 +28,30 @@ async def test_global_search_endpoint_scopes_every_page_to_an_exact_repository(m
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_global_search_endpoint_forwards_independent_stream_continuation(monkeypatch):
|
||||
requested = []
|
||||
|
||||
async def search(query, limit, page, kind, state, repository, continuation):
|
||||
requested.append((query, limit, page, kind, state, repository, continuation))
|
||||
return {
|
||||
"items": [], "partial": False, "has_more": False, "next_page": 3,
|
||||
"continuation": {"issues": None, "pulls": None}, "failed_streams": [],
|
||||
}
|
||||
|
||||
monkeypatch.setattr(main.gitea_proxy, "global_search", search, raising=False)
|
||||
transport = httpx.ASGITransport(app=main.app)
|
||||
async with httpx.AsyncClient(transport=transport, base_url="http://test") as client:
|
||||
response = await client.get(
|
||||
"/api/v1/search?q=mobile&limit=7&page=2&issues_page=1&pulls_page=3"
|
||||
)
|
||||
|
||||
assert response.status_code == 200
|
||||
assert requested == [
|
||||
("mobile", 7, 2, "all", "all", None, {"issues": 1, "pulls": 3})
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_global_search_rejects_malformed_repository_scope_before_upstream(monkeypatch):
|
||||
called = False
|
||||
@@ -335,12 +359,11 @@ async def test_global_search_queries_issues_and_pulls_and_skips_unsafe_results()
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert {request["type"] for request in requests} == {"issues", "pulls"}
|
||||
assert all(
|
||||
request["q"] == "mobile queue"
|
||||
and request["limit"] == "4"
|
||||
and request["state"] == "all"
|
||||
for request in requests
|
||||
)
|
||||
assert {request["type"]: request["limit"] for request in requests} == {
|
||||
"issues": "4", "pulls": "3"
|
||||
}
|
||||
assert all(request["q"] == "mobile queue" and request["state"] == "all"
|
||||
for request in requests)
|
||||
assert results == {
|
||||
"items": [{
|
||||
"kind": "issue", "repository": "stackchain/api", "number": 42,
|
||||
@@ -354,6 +377,8 @@ async def test_global_search_queries_issues_and_pulls_and_skips_unsafe_results()
|
||||
"partial": False,
|
||||
"has_more": False,
|
||||
"next_page": 2,
|
||||
"continuation": {"issues": None, "pulls": None},
|
||||
"failed_streams": [],
|
||||
}
|
||||
|
||||
|
||||
@@ -386,11 +411,93 @@ async def test_global_search_returns_healthy_stream_when_other_stream_fails():
|
||||
"url": "https://forge.example/stackchain/web/pulls/9",
|
||||
}],
|
||||
"partial": True,
|
||||
"has_more": False,
|
||||
"has_more": True,
|
||||
"next_page": 2,
|
||||
"continuation": {"issues": 1, "pulls": None},
|
||||
"failed_streams": ["issue"],
|
||||
}
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_global_search_retries_a_failed_stream_without_advancing_its_page():
|
||||
requests = []
|
||||
issue_attempts = 0
|
||||
|
||||
async def handler(request):
|
||||
nonlocal issue_attempts
|
||||
params = dict(request.url.params)
|
||||
requests.append((params["type"], params["page"], params["limit"]))
|
||||
if params["type"] == "issues":
|
||||
issue_attempts += 1
|
||||
if issue_attempts == 1:
|
||||
return httpx.Response(503)
|
||||
number = int(params["page"])
|
||||
return httpx.Response(200, json=[{
|
||||
"number": number,
|
||||
"title": f'{params["type"]} {number}',
|
||||
"state": "open",
|
||||
"repository": {"full_name": "stackchain/web"},
|
||||
"html_url": f'https://forge.example/stackchain/web/{params["type"]}/{number}',
|
||||
}])
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
partial = await gitea_proxy.global_search("search", limit=4)
|
||||
recovered = await gitea_proxy.global_search(
|
||||
"search", limit=4, continuation=partial["continuation"]
|
||||
)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
assert partial["partial"] is True
|
||||
assert partial["has_more"] is True
|
||||
assert partial["continuation"] == {"issues": 1, "pulls": None}
|
||||
assert recovered["partial"] is False
|
||||
assert [item["kind"] for item in recovered["items"]] == ["issue"]
|
||||
assert requests == [
|
||||
("issues", "1", "2"), ("pulls", "1", "2"), ("issues", "1", "2")
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_global_search_odd_limit_keeps_both_streams_contiguous_across_pages():
|
||||
requests = []
|
||||
|
||||
async def handler(request):
|
||||
params = dict(request.url.params)
|
||||
stream_limit = int(params["limit"])
|
||||
page = int(params["page"])
|
||||
requests.append((params["type"], page, stream_limit))
|
||||
start = (page - 1) * stream_limit + 1
|
||||
return httpx.Response(200, json=[{
|
||||
"number": number,
|
||||
"title": f'{params["type"]} {number}',
|
||||
"state": "open",
|
||||
"repository": {"full_name": "stackchain/web"},
|
||||
"html_url": f'https://forge.example/stackchain/web/{params["type"]}/{number}',
|
||||
} for number in range(start, start + stream_limit)])
|
||||
|
||||
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||
try:
|
||||
first = await gitea_proxy.global_search("search", limit=7)
|
||||
second = await gitea_proxy.global_search(
|
||||
"search", limit=7, continuation=first["continuation"]
|
||||
)
|
||||
finally:
|
||||
await gitea_proxy.stop_client()
|
||||
|
||||
pulls = [item["number"] for result in (first, second)
|
||||
for item in result["items"] if item["kind"] == "pull"]
|
||||
issues = [item["number"] for result in (first, second)
|
||||
for item in result["items"] if item["kind"] == "issue"]
|
||||
assert pulls == [1, 2, 3, 4, 5, 6]
|
||||
assert issues == [1, 2, 3, 4, 5, 6, 7, 8]
|
||||
assert requests == [
|
||||
("issues", 1, 4), ("pulls", 1, 3),
|
||||
("issues", 2, 4), ("pulls", 2, 3),
|
||||
]
|
||||
|
||||
|
||||
@pytest.mark.anyio
|
||||
async def test_global_search_caps_combined_results_to_requested_limit():
|
||||
async def handler(request):
|
||||
|
||||
Reference in new issue
Block a user