Merge pull request 'Keep global search useful during partial failures and rapid typing' (#200) from timmy/199-partial-cancellable-search into main
Keep global search useful during partial failures and rapid typing (#200) Closes #199
This commit is contained in:
commit
4cc00d2e48
|
|
@ -21,6 +21,7 @@
|
||||||
const delay = options.delay === undefined ? 250 : options.delay;
|
const delay = options.delay === undefined ? 250 : options.delay;
|
||||||
let timer = null;
|
let timer = null;
|
||||||
let generation = 0;
|
let generation = 0;
|
||||||
|
let activeController = null;
|
||||||
|
|
||||||
return {
|
return {
|
||||||
setQuery(value) {
|
setQuery(value) {
|
||||||
|
|
@ -28,17 +29,26 @@
|
||||||
generation += 1;
|
generation += 1;
|
||||||
const current = generation;
|
const current = generation;
|
||||||
if (timer !== null) clearTimeout(timer);
|
if (timer !== null) clearTimeout(timer);
|
||||||
|
if (activeController !== null) activeController.abort();
|
||||||
|
activeController = null;
|
||||||
if (query.length < 2) {
|
if (query.length < 2) {
|
||||||
onState({ status: 'idle', query, items: [] });
|
onState({ status: 'idle', query, items: [] });
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
onState({ status: 'loading', query, items: [] });
|
onState({ status: 'loading', query, items: [] });
|
||||||
timer = setTimeout(async () => {
|
timer = setTimeout(async () => {
|
||||||
|
const requestController = new AbortController();
|
||||||
|
activeController = requestController;
|
||||||
try {
|
try {
|
||||||
const items = await search(query);
|
const result = await search(query, requestController.signal);
|
||||||
if (current === generation) onState({ status: 'ready', query, items });
|
const items = Array.isArray(result) ? result : result.items;
|
||||||
|
const partial = !Array.isArray(result) && result.partial === true;
|
||||||
|
if (current === generation) onState({ status: 'ready', query, items, partial });
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
|
if (error && error.name === 'AbortError') return;
|
||||||
if (current === generation) onState({ status: 'error', query, items: [], error });
|
if (current === generation) onState({ status: 'error', query, items: [], error });
|
||||||
|
} finally {
|
||||||
|
if (activeController === requestController) activeController = null;
|
||||||
}
|
}
|
||||||
}, delay);
|
}, delay);
|
||||||
},
|
},
|
||||||
|
|
|
||||||
|
|
@ -1794,13 +1794,17 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
let commandSearchState = { status:'idle', query:'', items:[] };
|
let commandSearchState = { status:'idle', query:'', items:[] };
|
||||||
let commandItems = [];
|
let commandItems = [];
|
||||||
let commandSelection = -1;
|
let commandSelection = -1;
|
||||||
async function searchGlobalWork(query) {
|
async function searchGlobalWork(query, signal) {
|
||||||
const response = await fetch('api/v1/search?q=' + encodeURIComponent(query) + '&limit=10', {
|
const response = await fetch('api/v1/search?q=' + encodeURIComponent(query) + '&limit=10', {
|
||||||
headers: { Accept:'application/json' },
|
headers: { Accept:'application/json' },
|
||||||
|
signal,
|
||||||
});
|
});
|
||||||
const payload = await response.json().catch(() => ({}));
|
const payload = await response.json().catch(() => ({}));
|
||||||
if (!response.ok) throw new Error(payload.error || 'Search is temporarily unavailable.');
|
if (!response.ok) throw new Error(payload.error || 'Search is temporarily unavailable.');
|
||||||
return Array.isArray(payload.items) ? payload.items : [];
|
return {
|
||||||
|
items: Array.isArray(payload.items) ? payload.items : [],
|
||||||
|
partial: payload.partial === true,
|
||||||
|
};
|
||||||
}
|
}
|
||||||
const commandSearch = filterCommands.createGlobalSearchController({
|
const commandSearch = filterCommands.createGlobalSearchController({
|
||||||
search: searchGlobalWork,
|
search: searchGlobalWork,
|
||||||
|
|
@ -1920,6 +1924,7 @@ textarea { resize: vertical; min-height: 120px; }
|
||||||
}).join('');
|
}).join('');
|
||||||
if (commandSearchState.status === 'loading') html += '<div class="cmd-status">Searching accessible work…</div>';
|
if (commandSearchState.status === 'loading') html += '<div class="cmd-status">Searching accessible work…</div>';
|
||||||
else if (commandSearchState.status === 'error') html += '<div class="cmd-status">Search unavailable. Keep typing or retry.</div>';
|
else if (commandSearchState.status === 'error') html += '<div class="cmd-status">Search unavailable. Keep typing or retry.</div>';
|
||||||
|
else if (commandSearchState.partial) html += '<div class="cmd-status">Some results are temporarily unavailable.</div>';
|
||||||
else if (String(filter || '').trim().length >= 2 && !remote.length) html += '<div class="cmd-status">No matching issues or pull requests.</div>';
|
else if (String(filter || '').trim().length >= 2 && !remote.length) html += '<div class="cmd-status">No matching issues or pull requests.</div>';
|
||||||
el.innerHTML = html;
|
el.innerHTML = html;
|
||||||
el.querySelectorAll('.cmd-item').forEach((item) => {
|
el.querySelectorAll('.cmd-item').forEach((item) => {
|
||||||
|
|
|
||||||
|
|
@ -208,7 +208,7 @@ def _normalize_global_search_item(item: Any, kind: str) -> dict | None:
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
async def global_search(query: str, limit: int = 10) -> list[dict]:
|
async def global_search(query: str, limit: int = 10) -> dict:
|
||||||
"""Search accessible issues and pulls concurrently with a bounded result set."""
|
"""Search accessible issues and pulls concurrently with a bounded result set."""
|
||||||
async def load(item_type: str) -> Any:
|
async def load(item_type: str) -> Any:
|
||||||
response = await _get_client().get(
|
response = await _get_client().get(
|
||||||
|
|
@ -222,11 +222,21 @@ async def global_search(query: str, limit: int = 10) -> list[dict]:
|
||||||
raise ValueError("Gitea global search response was not a list")
|
raise ValueError("Gitea global search response was not a list")
|
||||||
return payload
|
return payload
|
||||||
|
|
||||||
issues_payload, pulls_payload = await asyncio.gather(load("issues"), load("pulls"))
|
outcomes = await asyncio.gather(
|
||||||
|
load("issues"), load("pulls"), return_exceptions=True
|
||||||
|
)
|
||||||
|
for outcome in outcomes:
|
||||||
|
if isinstance(outcome, asyncio.CancelledError):
|
||||||
|
raise outcome
|
||||||
|
if all(isinstance(outcome, BaseException) for outcome in outcomes):
|
||||||
|
raise outcomes[0]
|
||||||
|
|
||||||
results = []
|
results = []
|
||||||
seen: set[tuple[str, str, int]] = set()
|
seen: set[tuple[str, str, int]] = set()
|
||||||
for payload, kind in ((issues_payload, "issue"), (pulls_payload, "pull")):
|
for outcome, kind in zip(outcomes, ("issue", "pull"), strict=True):
|
||||||
for item in payload:
|
if isinstance(outcome, BaseException):
|
||||||
|
continue
|
||||||
|
for item in outcome:
|
||||||
normalized = _normalize_global_search_item(item, kind)
|
normalized = _normalize_global_search_item(item, kind)
|
||||||
if normalized is not None:
|
if normalized is not None:
|
||||||
identity = (kind, normalized["repository"], normalized["number"])
|
identity = (kind, normalized["repository"], normalized["number"])
|
||||||
|
|
@ -234,7 +244,10 @@ async def global_search(query: str, limit: int = 10) -> list[dict]:
|
||||||
continue
|
continue
|
||||||
seen.add(identity)
|
seen.add(identity)
|
||||||
results.append(normalized)
|
results.append(normalized)
|
||||||
return results
|
return {
|
||||||
|
"items": results[:limit],
|
||||||
|
"partial": any(isinstance(outcome, BaseException) for outcome in outcomes),
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
async def work_preview(repository: str, kind: str, number: int) -> dict:
|
async def work_preview(repository: str, kind: str, number: int) -> dict:
|
||||||
|
|
|
||||||
|
|
@ -481,7 +481,7 @@ async def global_search(
|
||||||
if len(query) < 2:
|
if len(query) < 2:
|
||||||
raise HTTPException(status_code=422, detail="Search query must contain at least 2 characters")
|
raise HTTPException(status_code=422, detail="Search query must contain at least 2 characters")
|
||||||
try:
|
try:
|
||||||
items = await asyncio.wait_for(
|
result = await asyncio.wait_for(
|
||||||
gitea_proxy.global_search(query, limit),
|
gitea_proxy.global_search(query, limit),
|
||||||
timeout=GLOBAL_SEARCH_TIMEOUT_SECONDS,
|
timeout=GLOBAL_SEARCH_TIMEOUT_SECONDS,
|
||||||
)
|
)
|
||||||
|
|
@ -491,7 +491,7 @@ async def global_search(
|
||||||
status_code=503,
|
status_code=503,
|
||||||
headers={"Retry-After": "1"},
|
headers={"Retry-After": "1"},
|
||||||
)
|
)
|
||||||
return JSONResponse({"query": query, "items": items})
|
return JSONResponse({"query": query, **result})
|
||||||
|
|
||||||
|
|
||||||
@app.get("/api/v1/repos/{owner}/{repo}/issues/{number}/preview")
|
@app.get("/api/v1/repos/{owner}/{repo}/issues/{number}/preview")
|
||||||
|
|
|
||||||
|
|
@ -82,6 +82,63 @@ if (ready[0].query !== 'release' || ready[0].items[0].title !== 'Current result'
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
def test_remote_command_search_aborts_superseded_and_cleared_queries():
|
||||||
|
script = f"""
|
||||||
|
const filterCommands = require({json.dumps(str(COMMANDS))});
|
||||||
|
(async () => {{
|
||||||
|
const signals = [];
|
||||||
|
const states = [];
|
||||||
|
const controller = filterCommands.createGlobalSearchController({{
|
||||||
|
delay: 0,
|
||||||
|
search: (query, signal) => {{
|
||||||
|
signals.push(signal);
|
||||||
|
return new Promise((resolve, reject) => signal.addEventListener('abort', () => {{
|
||||||
|
const error = new Error('aborted');
|
||||||
|
error.name = 'AbortError';
|
||||||
|
reject(error);
|
||||||
|
}}));
|
||||||
|
}},
|
||||||
|
onState: state => states.push(state),
|
||||||
|
}});
|
||||||
|
controller.setQuery('mobile');
|
||||||
|
await new Promise(resolve => setTimeout(resolve, 0));
|
||||||
|
controller.setQuery('release');
|
||||||
|
if (!signals[0].aborted) throw new Error('superseded request was not aborted');
|
||||||
|
await new Promise(resolve => setTimeout(resolve, 0));
|
||||||
|
controller.setQuery('');
|
||||||
|
if (!signals[1].aborted) throw new Error('cleared query request was not aborted');
|
||||||
|
await new Promise(resolve => setTimeout(resolve, 0));
|
||||||
|
if (states.some(state => state.status === 'error')) throw new Error('abort rendered an error');
|
||||||
|
if (states.at(-1).status !== 'idle') throw new Error('clear did not restore idle state');
|
||||||
|
}})().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_preserves_partial_result_status():
|
||||||
|
script = f"""
|
||||||
|
const filterCommands = require({json.dumps(str(COMMANDS))});
|
||||||
|
(async () => {{
|
||||||
|
const states = [];
|
||||||
|
const controller = filterCommands.createGlobalSearchController({{
|
||||||
|
delay: 0,
|
||||||
|
search: () => Promise.resolve({{ items:[{{ title:'Useful result' }}], partial:true }}),
|
||||||
|
onState: state => states.push(state),
|
||||||
|
}});
|
||||||
|
controller.setQuery('mobile');
|
||||||
|
await new Promise(resolve => setTimeout(resolve, 0));
|
||||||
|
await new Promise(resolve => setTimeout(resolve, 0));
|
||||||
|
const ready = states.find(state => state.status === 'ready');
|
||||||
|
if (!ready || ready.items.length !== 1 || ready.partial !== true) {{
|
||||||
|
throw new Error('partial result metadata was lost: ' + JSON.stringify(states));
|
||||||
|
}}
|
||||||
|
}})().catch(error => {{ console.error(error); process.exit(1); }});
|
||||||
|
"""
|
||||||
|
|
||||||
|
subprocess.run(["node", "-e", script], check=True, capture_output=True, text=True)
|
||||||
|
|
||||||
|
|
||||||
def test_command_selection_wraps_for_arrow_keys():
|
def test_command_selection_wraps_for_arrow_keys():
|
||||||
script = f"""
|
script = f"""
|
||||||
const commands = require({json.dumps(str(COMMANDS))});
|
const commands = require({json.dumps(str(COMMANDS))});
|
||||||
|
|
@ -101,6 +158,9 @@ def test_palette_exposes_accessible_global_work_search_under_dashboard_subpath()
|
||||||
assert 'aria-controls="cmd-results"' in html
|
assert 'aria-controls="cmd-results"' in html
|
||||||
assert 'role="listbox"' in html
|
assert 'role="listbox"' in html
|
||||||
assert "fetch('api/v1/search?q='" in html
|
assert "fetch('api/v1/search?q='" in html
|
||||||
|
assert "async function searchGlobalWork(query, signal)" in html
|
||||||
|
assert "signal," in html
|
||||||
|
assert "Some results are temporarily unavailable." in html
|
||||||
assert urljoin(
|
assert urljoin(
|
||||||
"https://forge.alexanderwhitestone.com/dashboard/",
|
"https://forge.alexanderwhitestone.com/dashboard/",
|
||||||
"api/v1/search?q=mobile",
|
"api/v1/search?q=mobile",
|
||||||
|
|
|
||||||
|
|
@ -1,3 +1,5 @@
|
||||||
|
import asyncio
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
import pytest
|
import pytest
|
||||||
|
|
||||||
|
|
@ -10,14 +12,17 @@ async def test_global_search_endpoint_returns_bounded_normalized_results(monkeyp
|
||||||
|
|
||||||
async def search(query, limit):
|
async def search(query, limit):
|
||||||
requested.append((query, limit))
|
requested.append((query, limit))
|
||||||
return [{
|
return {
|
||||||
|
"items": [{
|
||||||
"kind": "issue",
|
"kind": "issue",
|
||||||
"repository": "stackchain/api",
|
"repository": "stackchain/api",
|
||||||
"number": 42,
|
"number": 42,
|
||||||
"title": "Repair mobile queue",
|
"title": "Repair mobile queue",
|
||||||
"state": "open",
|
"state": "open",
|
||||||
"url": "https://forge.example/stackchain/api/issues/42",
|
"url": "https://forge.example/stackchain/api/issues/42",
|
||||||
}]
|
}],
|
||||||
|
"partial": False,
|
||||||
|
}
|
||||||
|
|
||||||
monkeypatch.setattr(main.gitea_proxy, "global_search", search, raising=False)
|
monkeypatch.setattr(main.gitea_proxy, "global_search", search, raising=False)
|
||||||
transport = httpx.ASGITransport(app=main.app)
|
transport = httpx.ASGITransport(app=main.app)
|
||||||
|
|
@ -34,7 +39,7 @@ async def test_global_search_endpoint_returns_bounded_normalized_results(monkeyp
|
||||||
"title": "Repair mobile queue",
|
"title": "Repair mobile queue",
|
||||||
"state": "open",
|
"state": "open",
|
||||||
"url": "https://forge.example/stackchain/api/issues/42",
|
"url": "https://forge.example/stackchain/api/issues/42",
|
||||||
}]}
|
}], "partial": False}
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
|
|
@ -106,7 +111,8 @@ async def test_global_search_queries_issues_and_pulls_and_skips_unsafe_results()
|
||||||
|
|
||||||
assert {request["type"] for request in requests} == {"issues", "pulls"}
|
assert {request["type"] for request in requests} == {"issues", "pulls"}
|
||||||
assert all(request["q"] == "mobile queue" and request["limit"] == "7" for request in requests)
|
assert all(request["q"] == "mobile queue" and request["limit"] == "7" for request in requests)
|
||||||
assert results == [{
|
assert results == {
|
||||||
|
"items": [{
|
||||||
"kind": "issue", "repository": "stackchain/api", "number": 42,
|
"kind": "issue", "repository": "stackchain/api", "number": 42,
|
||||||
"title": "Repair queue", "state": "open",
|
"title": "Repair queue", "state": "open",
|
||||||
"url": "https://forge.example/stackchain/api/issues/42",
|
"url": "https://forge.example/stackchain/api/issues/42",
|
||||||
|
|
@ -114,7 +120,79 @@ async def test_global_search_queries_issues_and_pulls_and_skips_unsafe_results()
|
||||||
"kind": "pull", "repository": "stackchain/web", "number": 9,
|
"kind": "pull", "repository": "stackchain/web", "number": 9,
|
||||||
"title": "Improve search", "state": "closed",
|
"title": "Improve search", "state": "closed",
|
||||||
"url": "https://forge.example/stackchain/web/pulls/9",
|
"url": "https://forge.example/stackchain/web/pulls/9",
|
||||||
}]
|
}],
|
||||||
|
"partial": False,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_global_search_returns_healthy_stream_when_other_stream_fails():
|
||||||
|
async def handler(request):
|
||||||
|
if request.url.params["type"] == "issues":
|
||||||
|
return httpx.Response(503, json={"message": "upstream details must stay private"})
|
||||||
|
return httpx.Response(200, json=[{
|
||||||
|
"number": 9,
|
||||||
|
"title": "Improve search",
|
||||||
|
"state": "open",
|
||||||
|
"repository": {"full_name": "stackchain/web"},
|
||||||
|
"html_url": "https://forge.example/stackchain/web/pulls/9",
|
||||||
|
}])
|
||||||
|
|
||||||
|
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||||
|
try:
|
||||||
|
result = await gitea_proxy.global_search("search", 10)
|
||||||
|
finally:
|
||||||
|
await gitea_proxy.stop_client()
|
||||||
|
|
||||||
|
assert result == {
|
||||||
|
"items": [{
|
||||||
|
"kind": "pull",
|
||||||
|
"repository": "stackchain/web",
|
||||||
|
"number": 9,
|
||||||
|
"title": "Improve search",
|
||||||
|
"state": "open",
|
||||||
|
"url": "https://forge.example/stackchain/web/pulls/9",
|
||||||
|
}],
|
||||||
|
"partial": True,
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_global_search_caps_combined_results_to_requested_limit():
|
||||||
|
async def handler(request):
|
||||||
|
kind = request.url.params["type"]
|
||||||
|
items = [{
|
||||||
|
"number": number,
|
||||||
|
"title": f"{kind} {number}",
|
||||||
|
"state": "open",
|
||||||
|
"repository": {"full_name": "stackchain/web"},
|
||||||
|
"html_url": f"https://forge.example/stackchain/web/{kind}/{number}",
|
||||||
|
} for number in (1, 2)]
|
||||||
|
return httpx.Response(200, json=items)
|
||||||
|
|
||||||
|
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||||
|
try:
|
||||||
|
result = await gitea_proxy.global_search("search", 2)
|
||||||
|
finally:
|
||||||
|
await gitea_proxy.stop_client()
|
||||||
|
|
||||||
|
assert len(result["items"]) == 2
|
||||||
|
assert result["partial"] is False
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.anyio
|
||||||
|
async def test_global_search_propagates_cancellation_to_stop_obsolete_work():
|
||||||
|
async def handler(request):
|
||||||
|
if request.url.params["type"] == "issues":
|
||||||
|
raise asyncio.CancelledError()
|
||||||
|
return httpx.Response(200, json=[])
|
||||||
|
|
||||||
|
gitea_proxy.start_client(transport=httpx.MockTransport(handler))
|
||||||
|
try:
|
||||||
|
with pytest.raises(asyncio.CancelledError):
|
||||||
|
await gitea_proxy.global_search("obsolete", 10)
|
||||||
|
finally:
|
||||||
|
await gitea_proxy.stop_client()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.anyio
|
@pytest.mark.anyio
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user