diff --git a/frontend/commands.js b/frontend/commands.js index 569a071..e2ca1ee 100644 --- a/frontend/commands.js +++ b/frontend/commands.js @@ -15,6 +15,15 @@ return current; }; + filterCommands.searchUrl = function searchUrl(query, page, scope, continuation) { + const streamPages = continuation ? + (continuation.issues ? '&issues_page=' + encodeURIComponent(continuation.issues) : '') + + (continuation.pulls ? '&pulls_page=' + encodeURIComponent(continuation.pulls) : '') : ''; + return 'api/v1/search?q=' + encodeURIComponent(query) + '&limit=10&page=' + page + + '&kind=' + encodeURIComponent(scope.kind) + '&state=' + encodeURIComponent(scope.state) + + (scope.repository ? '&repository=' + encodeURIComponent(scope.repository) : '') + streamPages; + }; + filterCommands.createGlobalSearchController = function createGlobalSearchController(options) { const search = options.search; const onState = options.onState; @@ -31,12 +40,14 @@ return state; } - async function requestPage(query, page, current, append) { + async function requestPage(query, page, current, append, continuation) { const requestController = new AbortController(); activeController = requestController; try { const requestScope = { ...scope }; - const result = await search(query, requestController.signal, page, requestScope); + const result = await search( + query, requestController.signal, page, requestScope, continuation + ); const partial = result.partial === true; const incoming = result.items || result; const combined = append ? state.items.concat(incoming) : incoming; @@ -47,6 +58,8 @@ status: 'ready', query, items, partial, more: !!result.has_more, next: result.next_page, + continuation:result.continuation, + failedStreams:result.failed_streams || [], scope:requestScope, }); } catch (error) { @@ -93,7 +106,7 @@ if (state.status !== 'ready' || !state.more || activeController) return Promise.resolve(state); const current = generation; const page = state.next; - return requestPage(state.query, page, current, true); + return requestPage(state.query, page, current, true, state.continuation); }, }; }; diff --git a/frontend/dashboard.js b/frontend/dashboard.js index a6f00c7..a0c4a38 100644 --- a/frontend/dashboard.js +++ b/frontend/dashboard.js @@ -4293,10 +4293,8 @@ let commandSearchState = { status:'idle', query:'', items:[] }; let commandItems = []; let commandSelection = -1; - async function searchGlobalWork(query, signal, page = 1, scope = {kind:'all', state:'all'}) { - const response = await fetch('api/v1/search?q=' + encodeURIComponent(query) + '&limit=10&page=' + page + - '&kind=' + encodeURIComponent(scope.kind) + '&state=' + encodeURIComponent(scope.state) + - (scope.repository ? '&repository=' + encodeURIComponent(scope.repository) : ''), { + async function searchGlobalWork(query, signal, page = 1, scope = {kind:'all', state:'all'}, continuation) { + const response = await fetch(filterCommands.searchUrl(query, page, scope, continuation), { headers: { Accept:'application/json' }, signal, }); @@ -4531,6 +4529,7 @@ else if (String(filter || '').trim().length >= 2 && !remote.length) html += '
No matching issues or pull requests.
'; el.innerHTML = html; loadMore.hidden = !commandSearchState.more; + loadMore.textContent = commandSearchState.partial ? 'Retry missing results' : 'More results'; el.querySelectorAll('.cmd-item').forEach((item) => { item.addEventListener('click', () => runCommandItem(commandItems[Number(item.dataset.idx)])); }); diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 1482496..b79d807 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -470,16 +470,31 @@ async def global_search( kind: str = "all", state: str = "all", repository: str | None = None, + continuation: dict[str, int | None] | None = None, ) -> dict: - """Search accessible issues and pulls concurrently with balanced pagination.""" + """Search accessible work with lossless, independently retryable streams.""" item_types = ("issues", "pulls") if kind == "all" else ( "issues" if kind == "issue" else "pulls", ) - stream_limit = (limit + 1) // 2 if kind == "all" else limit + stream_limits = { + item_type: ( + (limit + 1) // 2 if item_type == "issues" else limit // 2 + ) if kind == "all" else limit + for item_type in item_types + } + stream_pages = { + item_type: continuation.get(item_type) if continuation is not None else page + for item_type in item_types + } + active_types = tuple( + item_type for item_type in item_types + if stream_pages[item_type] is not None and stream_limits[item_type] > 0 + ) + async def load(item_type: str) -> Any: params = { "q": query, "type": item_type, "state": state, - "limit": stream_limit, "page": page, + "limit": stream_limits[item_type], "page": stream_pages[item_type], } if repository: owner, repo = repository.split("/", 1) @@ -495,16 +510,20 @@ async def global_search( raise ValueError("Gitea global search response was not a list") return payload - outcomes = await asyncio.gather(*(load(item_type) for item_type in item_types), return_exceptions=True) + outcomes = await asyncio.gather( + *(load(item_type) for item_type in active_types), return_exceptions=True + ) for outcome in outcomes: if isinstance(outcome, asyncio.CancelledError): raise outcome - if all(isinstance(outcome, BaseException) for outcome in outcomes): + if outcomes and all(isinstance(outcome, BaseException) for outcome in outcomes): raise outcomes[0] streams: list[list[dict]] = [] seen: set[tuple[str, str, int]] = set() - result_kinds = tuple("issue" if item_type == "issues" else "pull" for item_type in item_types) + result_kinds = tuple( + "issue" if item_type == "issues" else "pull" for item_type in active_types + ) for outcome, result_kind in zip(outcomes, result_kinds, strict=True): normalized_stream = [] if isinstance(outcome, BaseException): @@ -524,14 +543,26 @@ async def global_search( for stream in streams: if index < len(stream): results.append(stream[index]) + + failed_streams = [ + "issue" if item_type == "issues" else "pull" + for item_type, outcome in zip(active_types, outcomes, strict=True) + if isinstance(outcome, BaseException) + ] + next_continuation = {item_type: None for item_type in item_types} + for item_type, outcome in zip(active_types, outcomes, strict=True): + if isinstance(outcome, BaseException): + next_continuation[item_type] = stream_pages[item_type] + elif len(outcome) >= stream_limits[item_type]: + next_continuation[item_type] = int(stream_pages[item_type]) + 1 + has_more = any(value is not None for value in next_continuation.values()) return { "items": results[:limit], - "partial": any(isinstance(outcome, BaseException) for outcome in outcomes), - "has_more": any( - not isinstance(outcome, BaseException) and len(outcome) >= stream_limit - for outcome in outcomes - ), + "partial": bool(failed_streams), + "has_more": has_more, "next_page": page + 1, + "continuation": next_continuation, + "failed_streams": failed_streams, } diff --git a/src/main.py b/src/main.py index 7d5f05c..4c218fa 100644 --- a/src/main.py +++ b/src/main.py @@ -2732,15 +2732,25 @@ async def global_search( repository: str | None = Query( default=None, pattern=r"^[A-Za-z0-9_.-]+/[A-Za-z0-9_.-]+$", max_length=161 ), + issues_page: int | None = Query(default=None, ge=1, le=100), + pulls_page: int | None = Query(default=None, ge=1, le=100), ) -> JSONResponse: query = q.strip() if len(query) < 2: raise HTTPException(status_code=422, detail="Search query must contain at least 2 characters") try: arguments = (query, limit, page, kind, state) - operation = gitea_proxy.global_search(*arguments, repository) if repository else ( - gitea_proxy.global_search(*arguments) - ) + continuation = None + if issues_page is not None or pulls_page is not None: + continuation = {"issues": issues_page, "pulls": pulls_page} + if continuation is not None: + operation = gitea_proxy.global_search( + *arguments, repository, continuation + ) + elif repository: + operation = gitea_proxy.global_search(*arguments, repository) + else: + operation = gitea_proxy.global_search(*arguments) result = await asyncio.wait_for( operation, timeout=GLOBAL_SEARCH_TIMEOUT_SECONDS, diff --git a/tests/test_api_paths.py b/tests/test_api_paths.py index aaf3e4d..649839d 100644 --- a/tests/test_api_paths.py +++ b/tests/test_api_paths.py @@ -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 { diff --git a/tests/test_command_palette.py b/tests/test_command_palette.py index a5bf65e..0c820d6 100644 --- a/tests/test_command_palette.py +++ b/tests/test_command_palette.py @@ -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 '
Filter search results' 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 '' in html assert '' 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/", diff --git a/tests/test_global_search.py b/tests/test_global_search.py index a2588f8..bca5f2a 100644 --- a/tests/test_global_search.py +++ b/tests/test_global_search.py @@ -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):