Merge pull request 'Load pull workspaces through one authorized snapshot' (#1351) from timmy/1350-authorized-pull-snapshot into main

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")

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:

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):
+    pull = {"number": 7}
-        return False
 
-    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_snapshot", snapshot)
-    monkeypatch.setattr(main.gitea_proxy, "pull_workspace_capabilities", capabilities)
     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):
+    pull = {"number": 7}
-        return (repository, number) == ("stackchain/api", 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):
+    async def snapshot(repository, number):
-        return False
+        return {"pull": {}, "capabilities": {"authored": False, "assigned": False}}
 
-    async def capabilities(repository, number):
+    monkeypatch.setattr(main.gitea_proxy, "pull_workspace_snapshot", snapshot)
-        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)
     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):
+    async def snapshot(repository, number):
-        calls.append(("assigned", repository, number))
+        calls.append(("snapshot", repository, number))
-        return True
+        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 = []