From b47ed5d86eda9a8a8c8709a7459011f3e986df66 Mon Sep 17 00:00:00 2001 From: timmy Date: Mon, 24 Aug 2026 13:14:48 +0000 Subject: [PATCH] perf: reuse authorized pull snapshots (Closes #1350) --- src/gitea_proxy.py | 30 ++++++++-- src/main.py | 14 +++-- tests/test_pull_api.py | 125 +++++++++++++++++++++++++++++++++-------- 3 files changed, 137 insertions(+), 32 deletions(-) diff --git a/src/gitea_proxy.py b/src/gitea_proxy.py index 83244c8..829f7de 100644 --- a/src/gitea_proxy.py +++ b/src/gitea_proxy.py @@ -2836,6 +2836,22 @@ async def pull_workspace_capabilities(repository: str, number: int) -> dict[str, } +async def pull_workspace_snapshot(repository: str, number: int) -> dict: + """Resolve one canonical pull and the access granted by that exact snapshot.""" + login, pull = await _current_login_and_target( + f"repos/{repository}/pulls/{number}" + ) + author = pull.get("user") if isinstance(pull.get("user"), dict) else {} + open_pull = pull.get("state") == "open" and pull.get("merged") is not True + return { + "pull": pull, + "capabilities": { + "authored": open_pull and author.get("login", "").casefold() == login.casefold(), + "assigned": open_pull and _login_in_users(login, pull.get("assignees")), + }, + } + + async def is_assigned_pull(repository: str, number: int) -> bool: login, pull = await _current_login_and_target( f"repos/{repository}/pulls/{number}" @@ -2891,9 +2907,12 @@ async def update_authored_assigned_pull( } -async def pull_completion_detail(repository: str, number: int) -> dict: +async def pull_completion_detail( + repository: str, number: int, pull: dict | None = None +) -> dict: base = f"repos/{repository}/pulls/{number}" - pull = await fetch(base) + if pull is None: + pull = await fetch(base) if not isinstance(pull, dict): raise ValueError("Gitea pull request response was not an object") conversation = await issue_conversation_page(repository, number) @@ -3013,9 +3032,12 @@ def _normalize_reviewer_statuses(pull: dict, reviews: object, head_sha: str) -> return sorted(statuses.values(), key=lambda item: item["login"].casefold()) -async def pull_completion_review(repository: str, number: int) -> dict: +async def pull_completion_review( + repository: str, number: int, pull: dict | None = None +) -> dict: base = f"repos/{repository}/pulls/{number}" - pull = await fetch(base) + if pull is None: + pull = await fetch(base) if not isinstance(pull, dict): raise ValueError("Gitea pull request response was not an object") head_value = pull.get("head") diff --git a/src/main.py b/src/main.py index 85a7af6..bc9f337 100644 --- a/src/main.py +++ b/src/main.py @@ -6576,10 +6576,13 @@ async def assigned_pull_detail(owner: str, repo: str, number: int = PathParam(gt repository = f"{owner}/{repo}" async def load_assigned_pull(): - capabilities = await _pull_workspace_capabilities(repository, number) + snapshot = await gitea_proxy.pull_workspace_snapshot(repository, number) + capabilities = snapshot["capabilities"] if not _has_pull_workspace_access(capabilities): raise HTTPException(status_code=404, detail="Pull request not found") - detail = await gitea_proxy.pull_completion_detail(repository, number) + detail = await gitea_proxy.pull_completion_detail( + repository, number, snapshot["pull"] + ) return {**detail, "capabilities": capabilities} try: @@ -6673,10 +6676,13 @@ async def assigned_pull_review_data( repository = f"{owner}/{repo}" async def load_assigned_pull_review(): - capabilities = await _pull_workspace_capabilities(repository, number) + snapshot = await gitea_proxy.pull_workspace_snapshot(repository, number) + capabilities = snapshot["capabilities"] if not _has_pull_workspace_access(capabilities): raise HTTPException(status_code=404, detail="Pull request not found") - review = await gitea_proxy.pull_completion_review(repository, number) + review = await gitea_proxy.pull_completion_review( + repository, number, snapshot["pull"] + ) return {**review, "capabilities": capabilities} try: diff --git a/tests/test_pull_api.py b/tests/test_pull_api.py index 76f73a2..bcdc1bb 100644 --- a/tests/test_pull_api.py +++ b/tests/test_pull_api.py @@ -203,16 +203,54 @@ async def test_gitea_authored_unassigned_pull_has_workspace_access(): ] +@pytest.mark.anyio +async def test_authored_pull_detail_reuses_one_authorized_snapshot(): + requests = [] + + async def handler(request): + requests.append((request.method, request.url.path)) + if request.url.path == "/api/v1/user": + return httpx.Response(200, json={"login": "alex"}) + if request.url.path == "/api/v1/repos/stackchain/api/pulls/7": + return httpx.Response(200, json={ + "number": 7, + "title": "Ship mobile flow", + "body": "Ready for review", + "state": "open", + "merged": False, + "head": {"sha": "abc1234"}, + "user": {"login": "alex"}, + "assignees": [], + }) + if request.url.path == "/api/v1/repos/stackchain/api/issues/7/comments": + return httpx.Response(200, json=[], headers={"X-Total-Count": "0"}) + raise AssertionError(f"unexpected request: {request.method} {request.url.path}") + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get("/api/v1/repos/stackchain/api/pulls/7/detail") + finally: + await gitea_proxy.stop_client() + + assert response.status_code == 200 + assert response.json()["capabilities"] == {"authored": True, "assigned": False} + assert requests.count(("GET", "/api/v1/user")) == 1 + assert requests.count(("GET", "/api/v1/repos/stackchain/api/pulls/7")) == 1 + assert requests.count(("GET", "/api/v1/repos/stackchain/api/issues/7/comments")) == 1 + + @pytest.mark.anyio async def test_authored_unassigned_pull_detail_returns_capabilities(monkeypatch): - async def assigned(repository, number): - return False + pull = {"number": 7} - async def capabilities(repository, number): + async def snapshot(repository, number): assert (repository, number) == ("stackchain/api", 7) - return {"authored": True, "assigned": False} + return {"pull": pull, "capabilities": {"authored": True, "assigned": False}} - async def detail(repository, number): + async def detail(repository, number, authorized_pull): + assert authorized_pull is pull return { "repository": repository, "number": number, @@ -224,8 +262,7 @@ async def test_authored_unassigned_pull_detail_returns_capabilities(monkeypatch) "conversation": {"comments": [], "page": 1, "older_page": None, "total": 0}, } - monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned) - monkeypatch.setattr(main.gitea_proxy, "pull_workspace_capabilities", capabilities) + monkeypatch.setattr(main.gitea_proxy, "pull_workspace_snapshot", snapshot) monkeypatch.setattr(main.gitea_proxy, "pull_completion_detail", detail) transport = httpx.ASGITransport(app=main.app) async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: @@ -237,11 +274,15 @@ async def test_authored_unassigned_pull_detail_returns_capabilities(monkeypatch) @pytest.mark.anyio async def test_assigned_pull_detail_reports_completion_state(monkeypatch): - async def assigned(repository, number): - return (repository, number) == ("stackchain/api", 7) + pull = {"number": 7} - async def detail(repository, number): + async def snapshot(repository, number): assert (repository, number) == ("stackchain/api", 7) + return {"pull": pull, "capabilities": {"authored": False, "assigned": True}} + + async def detail(repository, number, authorized_pull): + assert (repository, number) == ("stackchain/api", 7) + assert authorized_pull is pull return { "repository": repository, "number": number, @@ -259,7 +300,7 @@ async def test_assigned_pull_detail_reports_completion_state(monkeypatch): "comments": [{"id": 9, "author": "sam", "body": "Ship it"}], } - monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned, raising=False) + monkeypatch.setattr(main.gitea_proxy, "pull_workspace_snapshot", snapshot) monkeypatch.setattr(main.gitea_proxy, "pull_completion_detail", detail, raising=False) transport = httpx.ASGITransport(app=main.app) async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: @@ -273,14 +314,10 @@ async def test_assigned_pull_detail_reports_completion_state(monkeypatch): @pytest.mark.anyio async def test_assigned_pull_detail_rejects_unassigned_pull(monkeypatch): - async def assigned(repository, number): - return False + async def snapshot(repository, number): + return {"pull": {}, "capabilities": {"authored": False, "assigned": False}} - async def capabilities(repository, number): - return {"authored": False, "assigned": False} - - monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned, raising=False) - monkeypatch.setattr(main.gitea_proxy, "pull_workspace_capabilities", capabilities) + monkeypatch.setattr(main.gitea_proxy, "pull_workspace_snapshot", snapshot) transport = httpx.ASGITransport(app=main.app) async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: response = await client.get("/api/v1/repos/private/secret/pulls/9/detail") @@ -330,12 +367,14 @@ async def test_gitea_assigned_pull_detail_loads_reading_without_review_resources @pytest.mark.anyio async def test_assigned_pull_review_endpoint_loads_review_payload_on_demand(monkeypatch): calls = [] + pull = {"number": 7} - async def assigned(repository, number): - calls.append(("assigned", repository, number)) - return True + async def snapshot(repository, number): + calls.append(("snapshot", repository, number)) + return {"pull": pull, "capabilities": {"authored": False, "assigned": True}} - async def review(repository, number): + async def review(repository, number, authorized_pull): + assert authorized_pull is pull calls.append(("review", repository, number)) return { "repository": repository, "number": number, "head_sha": "abc123", @@ -343,7 +382,7 @@ async def test_assigned_pull_review_endpoint_loads_review_payload_on_demand(monk "ci_state": "success", "files": [{"filename": "src/api.py"}], } - monkeypatch.setattr(main.gitea_proxy, "is_assigned_pull", assigned) + monkeypatch.setattr(main.gitea_proxy, "pull_workspace_snapshot", snapshot) monkeypatch.setattr(main.gitea_proxy, "pull_completion_review", review, raising=False) transport = httpx.ASGITransport(app=main.app) async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: @@ -353,11 +392,49 @@ async def test_assigned_pull_review_endpoint_loads_review_payload_on_demand(monk assert response.headers["cache-control"] == "no-store" assert response.json()["files"] == [{"filename": "src/api.py"}] assert calls == [ - ("assigned", "stackchain/api", 7), + ("snapshot", "stackchain/api", 7), ("review", "stackchain/api", 7), ] +@pytest.mark.anyio +async def test_assigned_pull_review_reuses_one_authorized_snapshot(): + requests = [] + + async def handler(request): + requests.append((request.method, request.url.path)) + if request.url.path == "/api/v1/user": + return httpx.Response(200, json={"login": "alex"}) + if request.url.path == "/api/v1/repos/stackchain/api/pulls/7": + return httpx.Response(200, json={ + "number": 7, "state": "open", "merged": False, "mergeable": True, + "head": {"sha": "abc1234"}, "user": {"login": "sam"}, + "assignees": [{"login": "alex"}], "requested_reviewers": [], + }) + if request.url.path == "/api/v1/repos/stackchain/api/pulls/7/files": + return httpx.Response(200, json=[]) + if request.url.path == "/api/v1/repos/stackchain/api/commits/abc1234/status": + return httpx.Response(200, json={"state": "pending", "statuses": []}) + if request.url.path == "/api/v1/repos/stackchain/api/pulls/7.diff": + return httpx.Response(200, text="") + if request.url.path == "/api/v1/repos/stackchain/api/pulls/7/reviews": + return httpx.Response(200, json=[]) + raise AssertionError(f"unexpected request: {request.method} {request.url.path}") + + gitea_proxy.start_client(transport=httpx.MockTransport(handler)) + try: + transport = httpx.ASGITransport(app=main.app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as client: + response = await client.get("/api/v1/repos/stackchain/api/pulls/7/review-data") + finally: + await gitea_proxy.stop_client() + + assert response.status_code == 200 + assert response.json()["capabilities"] == {"authored": False, "assigned": True} + assert requests.count(("GET", "/api/v1/user")) == 1 + assert requests.count(("GET", "/api/v1/repos/stackchain/api/pulls/7")) == 1 + + @pytest.mark.anyio async def test_assigned_pull_feedback_endpoint_authorizes_and_returns_one_review(monkeypatch): calls = [] -- 2.43.0