This is an automated email from the ASF dual-hosted git repository.
sbp pushed a commit to branch sbp
in repository https://gitbox.apache.org/repos/asf/tooling-trusted-releases.git
The following commit(s) were added to refs/heads/sbp by this push:
new 026834ba Add conditional queries to perform atomic vote transitions
026834ba is described below
commit 026834ba51aab39b9107823af447a25a55d313ec
Author: Sean B. Palmer <[email protected]>
AuthorDate: Fri Apr 3 20:22:54 2026 +0100
Add conditional queries to perform atomic vote transitions
---
atr/storage/writers/vote.py | 84 +++++++++++++++++++++++----
tests/unit/test_vote_resolution.py | 114 +++++++++++++++++++++++++++++++++++++
2 files changed, 187 insertions(+), 11 deletions(-)
diff --git a/atr/storage/writers/vote.py b/atr/storage/writers/vote.py
index fd21ff41..c2562d0b 100644
--- a/atr/storage/writers/vote.py
+++ b/atr/storage/writers/vote.py
@@ -21,6 +21,8 @@ from __future__ import annotations
import datetime
from typing import Literal
+import sqlmodel
+
import atr.construct as construct
import atr.db as db
import atr.db.interaction as interaction
@@ -312,8 +314,14 @@ class CommitteeMember(CommitteeParticipant):
match vote_result:
case "passed":
- release.phase = sql.ReleasePhase.RELEASE_PREVIEW
- release.vote_resolved = datetime.datetime.now(datetime.UTC)
+ await self._resolve_transition(
+ release,
+ expected_phase=sql.ReleasePhase.RELEASE_CANDIDATE,
+ expected_podling_thread_id=None,
+ new_phase=sql.ReleasePhase.RELEASE_PREVIEW,
+ new_vote_resolved=datetime.datetime.now(datetime.UTC),
+ new_podling_thread_id=None,
+ )
await self.__data.commit()
await self.__data.refresh(release)
success_message = "Vote marked as passed"
@@ -327,10 +335,15 @@ class CommitteeMember(CommitteeParticipant):
description=description,
)
case "failed" | "cancelled":
- release.phase = sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT
# The vote_resolved property refers to when the vote succeeded
only
- release.vote_resolved = None
- release.podling_thread_id = None
+ await self._resolve_transition(
+ release,
+ expected_phase=sql.ReleasePhase.RELEASE_CANDIDATE,
+ expected_podling_thread_id=None,
+ new_phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
+ new_vote_resolved=None,
+ new_podling_thread_id=None,
+ )
await self.__data.commit()
await self.__data.refresh(release)
success_message = f"Vote marked as {vote_result}"
@@ -369,7 +382,15 @@ class CommitteeMember(CommitteeParticipant):
if archive_url is None:
raise ValueError("No archive URL found for podling vote")
thread_id = archive_url.split("/")[-1]
- release.podling_thread_id = thread_id
+ await self._resolve_transition(
+ release,
+ expected_phase=sql.ReleasePhase.RELEASE_CANDIDATE,
+ expected_podling_thread_id=None,
+ new_phase=sql.ReleasePhase.RELEASE_CANDIDATE,
+ new_vote_resolved=None,
+ new_podling_thread_id=thread_id,
+ )
+ await self.__data.refresh(release)
# incubator_vote_address = "[email protected]"
incubator_vote_address = util.USER_TESTS_ADDRESS
if not release.project.committee:
@@ -407,8 +428,14 @@ class CommitteeMember(CommitteeParticipant):
)
success_message = "Project PPMC vote marked as passed, and
Incubator PMC vote automatically started"
elif vote_result == "passed":
- release.phase = sql.ReleasePhase.RELEASE_PREVIEW
- release.vote_resolved = datetime.datetime.now(datetime.UTC)
+ await self._resolve_transition(
+ release,
+ expected_phase=sql.ReleasePhase.RELEASE_CANDIDATE,
+ expected_podling_thread_id=release.podling_thread_id,
+ new_phase=sql.ReleasePhase.RELEASE_PREVIEW,
+ new_vote_resolved=datetime.datetime.now(datetime.UTC),
+ new_podling_thread_id=release.podling_thread_id,
+ )
await self.__data.commit()
await self.__data.refresh(release)
success_message = "Vote marked as passed"
@@ -427,10 +454,15 @@ class CommitteeMember(CommitteeParticipant):
)
extra_destination = (round_one_email_address,
round_one_message_id)
else:
- release.phase = sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT
# The vote_resolved property refers to when the vote succeeded only
- release.vote_resolved = None
- release.podling_thread_id = None
+ await self._resolve_transition(
+ release,
+ expected_phase=sql.ReleasePhase.RELEASE_CANDIDATE,
+ expected_podling_thread_id=release.podling_thread_id,
+ new_phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT,
+ new_vote_resolved=None,
+ new_podling_thread_id=None,
+ )
await self.__data.commit()
await self.__data.refresh(release)
success_message = f"Vote marked as {vote_result}"
@@ -532,6 +564,36 @@ class CommitteeMember(CommitteeParticipant):
await self.__data.commit()
return None
+ async def _resolve_transition(
+ self,
+ release: sql.Release,
+ *,
+ expected_phase: sql.ReleasePhase,
+ expected_podling_thread_id: str | None,
+ new_phase: sql.ReleasePhase,
+ new_vote_resolved: datetime.datetime | None,
+ new_podling_thread_id: str | None,
+ ) -> None:
+ via = sql.validate_instrumented_attribute
+ stmt = sqlmodel.update(sql.Release).where(
+ via(sql.Release.key) == release.key,
+ via(sql.Release.phase) == expected_phase,
+ )
+ if expected_podling_thread_id is None:
+ stmt = stmt.where(via(sql.Release.podling_thread_id).is_(None))
+ else:
+ stmt = stmt.where(via(sql.Release.podling_thread_id) ==
expected_podling_thread_id)
+ result = await self.__data.execute(
+ stmt.values(
+ phase=new_phase,
+ vote_resolved=new_vote_resolved,
+ podling_thread_id=new_podling_thread_id,
+ )
+ )
+ if getattr(result, "rowcount", 0) != 1:
+ await self.__data.rollback()
+ raise storage.AccessError("The release state has changed, please
refresh and try again")
+
# def __committee_member_or_admin(self, committee: sql.Committee, asf_uid:
str) -> None:
# if not (user.is_committee_member(committee, asf_uid) or
user.is_admin(asf_uid)):
# raise storage.AccessError("You do not have permission to perform
this action")
diff --git a/tests/unit/test_vote_resolution.py
b/tests/unit/test_vote_resolution.py
index d4e30df1..31cb2f6e 100644
--- a/tests/unit/test_vote_resolution.py
+++ b/tests/unit/test_vote_resolution.py
@@ -21,6 +21,7 @@ from types import SimpleNamespace
import pytest
import quart
+import sqlalchemy.engine as engine
import atr.db.interaction as interaction
import atr.get.manual as manual
@@ -77,6 +78,9 @@ async def
test_cancelled_resolve_release_clears_podling_thread_id() -> None:
release = _candidate_release(podling_thread_id="abc123")
data.merge = mock.AsyncMock(return_value=release)
+ data.refresh = _refresh_as(
+ phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT, vote_resolved=None,
podling_thread_id=None
+ )
await writer.resolve_release(
_project_key(),
@@ -100,6 +104,9 @@ async def
test_cancelled_resolve_release_produces_correct_message() -> None:
release = _candidate_release()
data.merge = mock.AsyncMock(return_value=release)
+ data.refresh = _refresh_as(
+ phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT, vote_resolved=None,
podling_thread_id=None
+ )
_release, _round, success, _error = await writer.resolve_release(
_project_key(),
@@ -123,6 +130,9 @@ async def test_cancelled_resolve_release_returns_to_draft()
-> None:
release = _candidate_release()
data.merge = mock.AsyncMock(return_value=release)
+ data.refresh = _refresh_as(
+ phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT, vote_resolved=None,
podling_thread_id=None
+ )
_release, _round, success, _error = await writer.resolve_release(
_project_key(),
@@ -149,6 +159,9 @@ async def
test_failed_resolve_release_clears_podling_thread_id() -> None:
release = _candidate_release(podling_thread_id="abc123")
data.merge = mock.AsyncMock(return_value=release)
+ data.refresh = _refresh_as(
+ phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT, vote_resolved=None,
podling_thread_id=None
+ )
await writer.resolve_release(
_project_key(),
@@ -176,6 +189,9 @@ async def
test_manual_cancelled_returns_to_draft_and_clears_podling_thread_id()
query = mock.MagicMock()
query.demand = mock.AsyncMock(return_value=release)
data.release = mock.MagicMock(return_value=query)
+ data.refresh = _refresh_as(
+ phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT, vote_resolved=None,
podling_thread_id=None
+ )
success = await writer.resolve_manually(
_project_key(),
@@ -201,6 +217,9 @@ async def
test_manual_failed_returns_to_draft_and_clears_podling_thread_id() ->
query = mock.MagicMock()
query.demand = mock.AsyncMock(return_value=release)
data.release = mock.MagicMock(return_value=query)
+ data.refresh = _refresh_as(
+ phase=sql.ReleasePhase.RELEASE_CANDIDATE_DRAFT, vote_resolved=None,
podling_thread_id=None
+ )
success = await writer.resolve_manually(
_project_key(),
@@ -225,6 +244,9 @@ async def test_manual_passed_creates_preview_revision() ->
None:
query = mock.MagicMock()
query.demand = mock.AsyncMock(return_value=release)
data.release = mock.MagicMock(return_value=query)
+ data.refresh = _refresh_as(
+ phase=sql.ReleasePhase.RELEASE_PREVIEW,
vote_resolved=datetime.datetime.now(datetime.UTC)
+ )
success = await writer.resolve_manually(
_project_key(),
@@ -264,6 +286,30 @@ async def
test_manual_resolve_page_explains_cancellation_notice_url(
assert "cancellation notice" in html
[email protected]
+async def test_manual_resolve_rejects_concurrent_modification() -> None:
+ """Manual resolve raises AccessError when the release was modified
concurrently."""
+ data = _mock_data()
+ write_as = _mock_write_as()
+ writer = _writer_with_mocks(data, write_as)
+
+ release = _manual_candidate_release()
+ query = mock.MagicMock()
+ query.demand = mock.AsyncMock(return_value=release)
+ data.release = mock.MagicMock(return_value=query)
+ data.execute = mock.AsyncMock(return_value=_mock_cursor_result(rowcount=0))
+
+ with pytest.raises(storage.AccessError, match="release state has changed"):
+ await writer.resolve_manually(
+ _project_key(),
+ _version_key(),
+ "passed",
+ )
+
+ data.rollback.assert_awaited()
+ write_as.revision.create_revision_with_quarantine.assert_not_awaited()
+
+
def test_manual_vote_resolve_section_links_to_manual_resolve(monkeypatch:
pytest.MonkeyPatch) -> None:
"""Manual vote releases link to the manual resolution page."""
@@ -290,6 +336,57 @@ def
test_manual_vote_resolve_section_links_to_manual_resolve(monkeypatch: pytest
assert 'href="/resolve/project/1.0.0"' not in html
[email protected]
+async def test_podling_double_pass_raises_error() -> None:
+ """A second podling round 1 pass raises AccessError when the first already
set podling_thread_id."""
+ data = _mock_data()
+ write_as = _mock_write_as()
+ writer = _writer_with_mocks(data, write_as)
+
+ release = _candidate_release()
+ data.merge = mock.AsyncMock(return_value=release)
+ data.execute = mock.AsyncMock(return_value=_mock_cursor_result(rowcount=0))
+ write_as.cache.get_message_archive_url =
mock.AsyncMock(return_value="https://lists.apache.org/thread/abc123";)
+
+ with pytest.raises(storage.AccessError, match="release state has changed"):
+ await writer.resolve_release(
+ _project_key(),
+ release,
+ 1,
+ "passed",
+ _latest_vote_task(),
+ "Chair",
+ "The vote has passed.",
+ )
+
+ data.rollback.assert_awaited()
+
+
[email protected]
+async def test_podling_stale_round_one_cancel_after_pass() -> None:
+ """A stale cancel after another user passes podling round 1 raises
AccessError."""
+ data = _mock_data()
+ write_as = _mock_write_as()
+ writer = _writer_with_mocks(data, write_as)
+
+ release = _candidate_release()
+ data.merge = mock.AsyncMock(return_value=release)
+ data.execute = mock.AsyncMock(return_value=_mock_cursor_result(rowcount=0))
+
+ with pytest.raises(storage.AccessError, match="release state has changed"):
+ await writer.resolve_release(
+ _project_key(),
+ release,
+ 1,
+ "cancelled",
+ _latest_vote_task(),
+ "Chair",
+ "The vote has been cancelled.",
+ )
+
+ data.rollback.assert_awaited()
+
+
@pytest.mark.asyncio
async def test_resolve_allows_cancelled_before_vote_end(monkeypatch:
pytest.MonkeyPatch) -> None:
"""Writer allows Cancelled even before the end of the vote."""
@@ -589,6 +686,7 @@ def _latest_vote_task_with_end(offset_hours: int) ->
SimpleNamespace:
def _manual_candidate_release(podling_thread_id: str | None = None) ->
SimpleNamespace:
return SimpleNamespace(
+ key="project-1.0.0",
phase=sql.ReleasePhase.RELEASE_CANDIDATE,
vote_manual=True,
vote_started=datetime.datetime.now(datetime.UTC),
@@ -607,12 +705,20 @@ def _manual_candidate_release(podling_thread_id: str |
None = None) -> SimpleNam
)
+def _mock_cursor_result(rowcount: int = 1) -> mock.MagicMock:
+ result = mock.MagicMock(spec=engine.CursorResult)
+ result.rowcount = rowcount
+ return result
+
+
def _mock_data() -> mock.MagicMock:
data = mock.MagicMock()
data.commit = mock.AsyncMock()
+ data.execute = mock.AsyncMock(return_value=_mock_cursor_result())
data.flush = mock.AsyncMock()
data.merge = mock.AsyncMock()
data.refresh = mock.AsyncMock()
+ data.rollback = mock.AsyncMock()
return data
@@ -627,6 +733,14 @@ def _project_key() -> safe.ProjectKey:
return safe.ProjectKey("project")
+def _refresh_as(**attrs: object) -> mock.AsyncMock:
+ def _apply(obj: SimpleNamespace) -> None:
+ for k, v in attrs.items():
+ setattr(obj, k, v)
+
+ return mock.AsyncMock(side_effect=_apply)
+
+
def _version_key() -> safe.VersionKey:
return safe.VersionKey("1.0.0")
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
