You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tooling.apache.org by sb...@apache.org on 2026/06/24 15:02:40 UTC
(tooling-trusted-releases) branch sbp updated: Fix a race condition in project deletion
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 770c39ee Fix a race condition in project deletion
770c39ee is described below
commit 770c39eee1f03ef8d116e84706d73d549c3235bb
Author: Sean B. Palmer <se...@miscoranda.com>
AuthorDate: Wed Jun 24 16:02:19 2026 +0100
Fix a race condition in project deletion
---
atr/storage/writers/project.py | 106 ++++++++++++++------------
tests/unit/test_project_writer_delete.py | 123 +++++++++++++++++++++++++++++++
2 files changed, 183 insertions(+), 46 deletions(-)
diff --git a/atr/storage/writers/project.py b/atr/storage/writers/project.py
index 86fe6767..69ba1012 100644
--- a/atr/storage/writers/project.py
+++ b/atr/storage/writers/project.py
@@ -217,64 +217,78 @@ class CommitteeMember(ReleaseManager):
)
async def archive(self, project_key: safe.ProjectKey) -> None:
- project = await self.__data.project(key=str(project_key), status=sql.ProjectStatus.ACTIVE, _releases=True).get()
-
- if not project:
- raise storage.AccessError(f"Project '{project_key}' not found.")
-
- # TODO: when a project has current or archived releases, archiving the project
- # should be allowed and should cascade-archive the current releases as well -
- # for now we just block, and only drafts are handled (deleted by the caller)
- post_draft_phases = {
- sql.ReleasePhase.RELEASE_CANDIDATE,
- sql.ReleasePhase.RELEASE_PREVIEW,
- sql.ReleasePhase.RELEASE,
- }
- if any(r.phase in post_draft_phases for r in project.releases_including_embargoed):
- raise storage.AccessError(
- f"Cannot archive project '{project_key}' because it has active releases; complete or remove them first."
- )
-
- if project.committee_key:
- committee_projects = await self.__data.project(
- committee_key=project.committee_key, status=sql.ProjectStatus.ACTIVE
- ).all()
- if len(committee_projects) <= 1:
+ await self.__data.begin_immediate()
+ self.__data.expire_all()
+ try:
+ project = await self.__data.project(
+ key=str(project_key), status=sql.ProjectStatus.ACTIVE, _releases=True
+ ).get()
+ if not project:
+ raise storage.AccessError(f"Project '{project_key}' not found.")
+
+ # TODO: when a project has current or archived releases, archiving the project
+ # should be allowed and should cascade-archive the current releases as well -
+ # for now we just block, and only drafts are handled (deleted by the caller)
+ post_draft_phases = {
+ sql.ReleasePhase.RELEASE_CANDIDATE,
+ sql.ReleasePhase.RELEASE_PREVIEW,
+ sql.ReleasePhase.RELEASE,
+ }
+ if any(r.phase in post_draft_phases for r in project.releases_including_embargoed):
raise storage.AccessError(
- f"Cannot archive project '{project_key}' because it is the only project in its committee."
+ f"Cannot archive project '{project_key}' because it has active releases;"
+ " complete or remove them first."
)
- project = await self.__data.merge(project)
- project.status = sql.ProjectStatus.RETIRED
- await self.__data.commit()
+ if project.committee_key:
+ committee_projects = await self.__data.project(
+ committee_key=project.committee_key, status=sql.ProjectStatus.ACTIVE
+ ).all()
+ if len(committee_projects) <= 1:
+ raise storage.AccessError(
+ f"Cannot archive project '{project_key}' because it is the only project in its committee."
+ )
+
+ project.status = sql.ProjectStatus.RETIRED
+ await self.__data.commit()
+ except Exception:
+ await self.__data.rollback()
+ raise
self.__write_as.append_to_audit_log(
asf_uid=self.__asf_uid,
project_key=str(project_key),
)
async def delete(self, project_key: safe.ProjectKey) -> None:
- project = await self.__data.project(key=str(project_key), status=sql.ProjectStatus.ACTIVE, _releases=True).get()
-
- if not project:
- raise storage.AccessError(f"Project '{project_key}' not found.", status=404)
-
- if project.committee_key:
- committee_projects = await self.__data.project(
- committee_key=project.committee_key, status=sql.ProjectStatus.ACTIVE
- ).all()
- if len(committee_projects) <= 1:
+ await self.__data.begin_immediate()
+ self.__data.expire_all()
+ try:
+ project = await self.__data.project(
+ key=str(project_key), status=sql.ProjectStatus.ACTIVE, _releases=True
+ ).get()
+ if not project:
+ raise storage.AccessError(f"Project '{project_key}' not found.", status=404)
+
+ if project.committee_key:
+ committee_projects = await self.__data.project(
+ committee_key=project.committee_key, status=sql.ProjectStatus.ACTIVE
+ ).all()
+ if len(committee_projects) <= 1:
+ raise storage.AccessError(
+ f"Cannot delete project '{project_key}' because it is the only project in its committee."
+ )
+
+ if project.releases_including_embargoed:
raise storage.AccessError(
- f"Cannot delete project '{project_key}' because it is the only project in its committee."
+ f"Cannot delete project '{project_key}' because it has associated releases. Delete them first.",
+ status=409,
)
- if project.releases_including_embargoed:
- raise storage.AccessError(
- f"Cannot delete project '{project_key}' because it has associated releases. Delete them first.",
- status=409,
- )
-
- await self.__data.delete(project)
- await self.__data.commit()
+ await self.__data.delete(project)
+ await self.__data.commit()
+ except Exception:
+ await self.__data.rollback()
+ raise
self.__write_as.append_to_audit_log(
asf_uid=self.__asf_uid,
project_key=str(project_key),
diff --git a/tests/unit/test_project_writer_delete.py b/tests/unit/test_project_writer_delete.py
new file mode 100644
index 00000000..ae829840
--- /dev/null
+++ b/tests/unit/test_project_writer_delete.py
@@ -0,0 +1,123 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+import unittest.mock as mock
+from types import SimpleNamespace
+
+import pytest
+
+import atr.models.safe as safe
+import atr.models.sql as sql
+import atr.storage as storage
+import atr.storage.writers.project as project
+
+
+@pytest.mark.asyncio
+async def test_archive_sole_active_project_locks_then_rolls_back() -> None:
+ target = SimpleNamespace(
+ key="alpha-one", committee_key="alpha", status=sql.ProjectStatus.ACTIVE, releases_including_embargoed=[]
+ )
+ data = mock.MagicMock()
+ data.begin_immediate = mock.AsyncMock()
+ data.rollback = mock.AsyncMock()
+ data.commit = mock.AsyncMock()
+ data.project = mock.MagicMock(side_effect=[_query(get=target), _query(all_=[target])])
+ writer = _writer(data)
+
+ with pytest.raises(storage.AccessError, match="only project"):
+ await writer.archive(safe.ProjectKey("alpha-one"))
+
+ data.begin_immediate.assert_awaited_once()
+ data.rollback.assert_awaited_once()
+ data.commit.assert_not_awaited()
+ assert target.status == sql.ProjectStatus.ACTIVE
+
+
+@pytest.mark.asyncio
+async def test_delete_non_sole_project_commits() -> None:
+ target = SimpleNamespace(key="alpha-one", committee_key="alpha", releases_including_embargoed=[])
+ other = SimpleNamespace(key="alpha-two", committee_key="alpha", releases_including_embargoed=[])
+ data = mock.MagicMock()
+ data.begin_immediate = mock.AsyncMock()
+ data.rollback = mock.AsyncMock()
+ data.commit = mock.AsyncMock()
+ data.delete = mock.AsyncMock()
+ data.project = mock.MagicMock(side_effect=[_query(get=target), _query(all_=[target, other])])
+ writer = _writer(data)
+
+ await writer.delete(safe.ProjectKey("alpha-one"))
+
+ data.begin_immediate.assert_awaited_once()
+ data.delete.assert_awaited_once_with(target)
+ data.commit.assert_awaited_once()
+ data.rollback.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_delete_sole_active_project_locks_then_rolls_back() -> None:
+ target = SimpleNamespace(key="alpha-one", committee_key="alpha", releases_including_embargoed=[])
+ data = mock.MagicMock()
+ data.begin_immediate = mock.AsyncMock()
+ data.rollback = mock.AsyncMock()
+ data.commit = mock.AsyncMock()
+ data.delete = mock.AsyncMock()
+ data.project = mock.MagicMock(side_effect=[_query(get=target), _query(all_=[target])])
+ writer = _writer(data)
+
+ with pytest.raises(storage.AccessError, match="only project"):
+ await writer.delete(safe.ProjectKey("alpha-one"))
+
+ data.begin_immediate.assert_awaited_once()
+ data.rollback.assert_awaited_once()
+ data.commit.assert_not_awaited()
+ data.delete.assert_not_awaited()
+
+
+@pytest.mark.asyncio
+async def test_delete_with_releases_rolls_back() -> None:
+ target = SimpleNamespace(key="alpha-one", committee_key="alpha", releases_including_embargoed=[object()])
+ other = SimpleNamespace(key="alpha-two", committee_key="alpha", releases_including_embargoed=[])
+ data = mock.MagicMock()
+ data.begin_immediate = mock.AsyncMock()
+ data.rollback = mock.AsyncMock()
+ data.commit = mock.AsyncMock()
+ data.delete = mock.AsyncMock()
+ data.project = mock.MagicMock(side_effect=[_query(get=target), _query(all_=[target, other])])
+ writer = _writer(data)
+
+ with pytest.raises(storage.AccessError, match="associated releases"):
+ await writer.delete(safe.ProjectKey("alpha-one"))
+
+ data.begin_immediate.assert_awaited_once()
+ data.rollback.assert_awaited_once()
+ data.commit.assert_not_awaited()
+ data.delete.assert_not_awaited()
+
+
+def _query(get: object = None, all_: list | None = None) -> mock.MagicMock:
+ query = mock.MagicMock()
+ query.get = mock.AsyncMock(return_value=get)
+ query.all = mock.AsyncMock(return_value=all_ or [])
+ return query
+
+
+def _writer(data: mock.MagicMock) -> project.CommitteeMember:
+ writer = object.__new__(project.CommitteeMember)
+ writer._CommitteeMember__data = data
+ writer._CommitteeMember__asf_uid = "tester"
+ writer._CommitteeMember__write_as = mock.MagicMock()
+ return writer
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@tooling.apache.org
For additional commands, e-mail: commits-help@tooling.apache.org