perf: reuse authorized pull snapshots (Closes #1350)
This commit is contained in:
parent
f0f415c65d
commit
b47ed5d86e
|
|
@ -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")
|
||||
|
|
|
|||
14
src/main.py
14
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:
|
||||
|
|
|
|||
|
|
@ -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 = []
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user