diff --git a/src/sentry/models/release.py b/src/sentry/models/release.py index a1c5d16f6d7e63..23deab547d01f2 100644 --- a/src/sentry/models/release.py +++ b/src/sentry/models/release.py @@ -32,6 +32,7 @@ from sentry.db.models.indexes import IndexWithPostgresNameLimits from sentry.db.models.manager.base import BaseManager from sentry.models.artifactbundle import ArtifactBundle +from sentry.models.commit import Commit from sentry.models.commitauthor import CommitAuthor from sentry.models.releases.constants import ( DB_VERSION_LENGTH, @@ -41,7 +42,6 @@ from sentry.models.releases.exceptions import UnsafeReleaseDeletion from sentry.models.releases.release_project import ReleaseProject from sentry.models.releases.util import ReleaseQuerySet, SemverFilter, SemverVersion -from sentry.releases.commits import get_or_create_commit from sentry.utils import metrics from sentry.utils.cache import cache from sentry.utils.db import atomic_transaction @@ -618,8 +618,8 @@ def set_refs(self, refs, user_id, fetch=False): for ref in refs: repo = repos_by_name[ref["repository"]] - commit = get_or_create_commit( - organization=self.organization, repo_id=repo.id, key=ref["commit"] + commit = Commit.objects.get_or_create( + organization_id=self.organization_id, repository_id=repo.id, key=ref["commit"] )[0] # update head commit for repo/release if exists ReleaseHeadCommit.objects.create_or_update( diff --git a/src/sentry/models/releases/set_commits.py b/src/sentry/models/releases/set_commits.py index 04ee7340c81604..3a21532f99ade8 100644 --- a/src/sentry/models/releases/set_commits.py +++ b/src/sentry/models/releases/set_commits.py @@ -11,6 +11,7 @@ from sentry.db.postgres.transactions import in_test_hide_transaction_boundary from sentry.locks import locks from sentry.models.activity import Activity +from sentry.models.commit import Commit from sentry.models.commitauthor import CommitAuthor from sentry.models.commitfilechange import CommitFileChange from sentry.models.grouphistory import GroupHistoryStatus, record_group_history @@ -34,7 +35,6 @@ from sentry.models.releaseheadcommit import ReleaseHeadCommit from sentry.models.repository import Repository from sentry.plugins.providers.repository import RepositoryProvider -from sentry.releases.commits import get_or_create_commit class _CommitDataKwargs(TypedDict, total=False): @@ -123,15 +123,11 @@ def set_commit(idx, data, release): if "timestamp" in data: commit_data["date_added"] = data["timestamp"] - commit, new_commit, created = get_or_create_commit( - organization=release.organization, - repo_id=repo.id, + commit, created = Commit.objects.get_or_create( + organization_id=release.organization_id, + repository_id=repo.id, key=data["id"], - message=commit_data.get("message"), - author=commit_data.get("author"), - # XXX: This code was in place before and passes either a string or datetime, but - # works ok. Just skipping the type checking - date_added=commit_data.get("date_added"), # type: ignore[arg-type] + defaults=commit_data, ) if not created and any(getattr(commit, key) != value for key, value in commit_data.items()): commit.update(**commit_data) diff --git a/src/sentry/releases/commits.py b/src/sentry/releases/commits.py index dc995ae341e5ea..2c37b74845487b 100644 --- a/src/sentry/releases/commits.py +++ b/src/sentry/releases/commits.py @@ -65,39 +65,3 @@ def create_commit( ) new_commit = _dual_write_commit(old_commit) return old_commit, new_commit - - -def get_or_create_commit( - organization: Organization, - repo_id: int, - key: str, - message: str | None = None, - author: CommitAuthor | None = None, - date_added: datetime | None = None, -) -> tuple[OldCommit, Commit, bool]: - """ - Gets or creates a commit with dual write support. - """ - defaults = { - "author": author, - "message": message, - } - if date_added is not None: - defaults["date_added"] = date_added # type: ignore[assignment] - - with atomic_transaction( - using=( - router.db_for_write(OldCommit), - router.db_for_write(Commit), - ) - ): - old_commit, created = OldCommit.objects.get_or_create( - organization_id=organization.id, - repository_id=repo_id, - key=key, - defaults=defaults, - ) - - new_commit = _dual_write_commit(old_commit) - - return old_commit, new_commit, created diff --git a/tests/sentry/releases/test_commits.py b/tests/sentry/releases/test_commits.py index 0fb65778d7f168..d366454f0ca434 100644 --- a/tests/sentry/releases/test_commits.py +++ b/tests/sentry/releases/test_commits.py @@ -7,7 +7,7 @@ from sentry.models.commit import Commit as OldCommit from sentry.models.commitauthor import CommitAuthor from sentry.models.repository import Repository -from sentry.releases.commits import create_commit, get_or_create_commit +from sentry.releases.commits import create_commit from sentry.releases.models import Commit from sentry.testutils.cases import TestCase @@ -143,90 +143,3 @@ def test_create_commit_transaction_atomicity(self): ) assert not OldCommit.objects.filter(key="test_atomicity_key").exists() assert not Commit.objects.filter(key="test_atomicity_key").exists() - - -class GetOrCreateCommitDualWriteTest(TestCase): - def setUp(self): - super().setUp() - self.repo = Repository.objects.create( - name="test-repo", - organization_id=self.organization.id, - ) - self.author = CommitAuthor.objects.create( - organization_id=self.organization.id, - email="test@example.com", - name="Test Author", - ) - - def test_get_or_create_commit_creates_new(self): - """Test that get_or_create creates a new commit when it doesn't exist""" - old_commit, new_commit, created = get_or_create_commit( - organization=self.organization, - repo_id=self.repo.id, - key="new123", - message="New commit message", - author=self.author, - ) - - assert created is True - assert old_commit.key == "new123" - assert old_commit.message == "New commit message" - assert old_commit.author == self.author - - assert new_commit is not None - assert new_commit.id == old_commit.id - assert new_commit.key == "new123" - assert new_commit.message == "New commit message" - assert new_commit.author == self.author - - def test_get_or_create_commit_gets_existing(self): - """Test that get_or_create returns existing commit when it exists""" - existing_old, existing_new = create_commit( - organization=self.organization, - repo_id=self.repo.id, - key="existing456", - message="Existing commit", - author=self.author, - ) - assert existing_new is not None - old_commit, new_commit, created = get_or_create_commit( - organization=self.organization, - repo_id=self.repo.id, - key="existing456", - message="This should not be used", - author=None, - ) - assert created is False - assert old_commit.id == existing_old.id - assert old_commit.key == "existing456" - assert old_commit.message == "Existing commit" - assert old_commit.author == self.author - assert new_commit is not None - assert new_commit.id == existing_new.id - - def test_get_or_create_commit_backfills_to_new_table(self): - """Test that get_or_create backfills to new table if commit exists only in old table""" - old_only_commit = OldCommit.objects.create( - organization_id=self.organization.id, - repository_id=self.repo.id, - key="old_only789", - message="Old table only", - author=self.author, - ) - assert not Commit.objects.filter(id=old_only_commit.id).exists() - - old_commit, new_commit, created = get_or_create_commit( - organization=self.organization, - repo_id=self.repo.id, - key="old_only789", - message="Should not be used", - ) - - assert created is False - assert old_commit.id == old_only_commit.id - assert old_commit.message == "Old table only" - assert new_commit is not None - assert new_commit.id == old_only_commit.id - assert new_commit.key == "old_only789" - assert new_commit.message == "Old table only" - assert new_commit.author == self.author