diff --git a/src/sentry/backup/comparators.py b/src/sentry/backup/comparators.py index f184c92e6782a5..d0dad3fe263f06 100644 --- a/src/sentry/backup/comparators.py +++ b/src/sentry/backup/comparators.py @@ -451,7 +451,11 @@ def get_default_comparators(): "sentry.querysubscription": [DateUpdatedComparator("date_updated")], "sentry.relay": [HashObfuscatingComparator("relay_id", "public_key")], "sentry.relayusage": [HashObfuscatingComparator("relay_id", "public_key")], - "sentry.sentryapp": [EmailObfuscatingComparator("creator_label")], + "sentry.sentryapp": [ + DateUpdatedComparator("date_updated"), + EmailObfuscatingComparator("creator_label"), + ], + "sentry.sentryappinstallation": [DateUpdatedComparator("date_updated")], "sentry.servicehook": [HashObfuscatingComparator("secret")], "sentry.user": [HashObfuscatingComparator("password")], "sentry.useremail": [ diff --git a/src/sentry/backup/imports.py b/src/sentry/backup/imports.py index 09fab2bbe6e608..93042327eb7a3d 100644 --- a/src/sentry/backup/imports.py +++ b/src/sentry/backup/imports.py @@ -149,7 +149,7 @@ def _import( if f.model == type(o) and getattr(o, f.field, None) not in f.values: break else: - written = o.write_relocation_import(pk_map, obj, scope) + written = o.write_relocation_import(pk_map, scope) if written is not None: old_pk, new_pk = written pk_map.insert(model_name, old_pk, new_pk) diff --git a/src/sentry/backup/mixins.py b/src/sentry/backup/mixins.py index a877beeaabf5f3..41615c903c2a57 100644 --- a/src/sentry/backup/mixins.py +++ b/src/sentry/backup/mixins.py @@ -2,8 +2,6 @@ from typing import Optional, Tuple -from django.core.serializers.base import DeserializedObject - from sentry.backup.dependencies import PrimaryKeyMap from sentry.backup.scopes import ImportScope @@ -19,9 +17,9 @@ class SanitizeUserImportsMixin: """ def write_relocation_import( - self, pk_map: PrimaryKeyMap, obj: DeserializedObject, scope: ImportScope + self, pk_map: PrimaryKeyMap, scope: ImportScope ) -> Optional[Tuple[int, int]]: if scope != ImportScope.Global: return None - return super().write_relocation_import(pk_map, obj, scope) # type: ignore[misc] + return super().write_relocation_import(pk_map, scope) # type: ignore[misc] diff --git a/src/sentry/db/models/base.py b/src/sentry/db/models/base.py index 286babc54c3fd3..3301838c45f747 100644 --- a/src/sentry/db/models/base.py +++ b/src/sentry/db/models/base.py @@ -3,7 +3,6 @@ from typing import Any, Callable, Iterable, Mapping, Optional, Tuple, Type, TypeVar from django.apps.config import AppConfig -from django.core.serializers.base import DeserializedObject from django.db import models from django.db.models import signals from django.utils import timezone @@ -139,7 +138,7 @@ def _normalize_before_relocation_import( return old_pk def write_relocation_import( - self, pk_map: PrimaryKeyMap, obj: DeserializedObject, scope: ImportScope + self, pk_map: PrimaryKeyMap, scope: ImportScope ) -> Optional[Tuple[int, int]]: """ Writes a deserialized model to the database. If this write is successful, this method will return a tuple of the old and new `pk`s. @@ -149,7 +148,7 @@ def write_relocation_import( if old_pk is None: return - obj.save(force_insert=True) + self.save(force_insert=True) return (old_pk, self.pk) diff --git a/src/sentry/models/options/project_option.py b/src/sentry/models/options/project_option.py index 52a6218e534bfc..e19bff1e786e2f 100644 --- a/src/sentry/models/options/project_option.py +++ b/src/sentry/models/options/project_option.py @@ -1,11 +1,12 @@ from __future__ import annotations -from typing import TYPE_CHECKING, Any, Mapping, Sequence +from typing import TYPE_CHECKING, Any, Mapping, Optional, Sequence, Tuple from django.db import models from sentry import projectoptions -from sentry.backup.scopes import RelocationScope +from sentry.backup.dependencies import PrimaryKeyMap +from sentry.backup.scopes import ImportScope, RelocationScope from sentry.db.models import FlexibleForeignKey, Model, region_silo_only_model, sane_repr from sentry.db.models.fields import PickledObjectField from sentry.db.models.manager import OptionManager, ValidateFunction, Value @@ -159,3 +160,19 @@ class Meta: unique_together = (("project", "key"),) __repr__ = sane_repr("project_id", "key", "value") + + def write_relocation_import( + self, pk_map: PrimaryKeyMap, scope: ImportScope + ) -> Optional[Tuple[int, int]]: + old_pk = super()._normalize_before_relocation_import(pk_map, scope) + if old_pk is None: + return None + + (key, _) = self.__class__.objects.get_or_create( + project=self.project, key=self.key, defaults={"value": self.value} + ) + if key: + self.pk = key.pk + self.save() + + return (old_pk, self.pk) diff --git a/src/sentry/models/projectkey.py b/src/sentry/models/projectkey.py index 2e4162d30789b6..e56c53bf5a2060 100644 --- a/src/sentry/models/projectkey.py +++ b/src/sentry/models/projectkey.py @@ -2,19 +2,21 @@ import re import secrets -from typing import Any +from typing import Any, Optional, Tuple from urllib.parse import urlparse import petname from django.conf import settings from django.db import ProgrammingError, models +from django.forms import model_to_dict from django.urls import reverse from django.utils import timezone from django.utils.translation import gettext_lazy as _ from bitfield import TypedClassBitField from sentry import features, options -from sentry.backup.scopes import RelocationScope +from sentry.backup.dependencies import PrimaryKeyMap +from sentry.backup.scopes import ImportScope, RelocationScope from sentry.db.models import ( BaseManager, BoundedPositiveIntegerField, @@ -276,3 +278,19 @@ def get_audit_log_data(self): def get_scopes(self): return self.scopes + + def write_relocation_import( + self, pk_map: PrimaryKeyMap, scope: ImportScope + ) -> Optional[Tuple[int, int]]: + old_pk = super()._normalize_before_relocation_import(pk_map, scope) + if old_pk is None: + return None + + (key, _) = self.__class__.objects.get_or_create( + project=self.project, defaults=model_to_dict(self) + ) + if key: + self.pk = key.pk + self.save() + + return (old_pk, self.pk) diff --git a/src/sentry/models/team.py b/src/sentry/models/team.py index 8feb8b796a1179..e11d34133932fc 100644 --- a/src/sentry/models/team.py +++ b/src/sentry/models/team.py @@ -5,7 +5,6 @@ from typing import TYPE_CHECKING, Literal, Optional, Sequence, Tuple, Union, overload from django.conf import settings -from django.core.serializers.base import DeserializedObject from django.db import IntegrityError, connections, models, router, transaction from django.utils import timezone from django.utils.translation import gettext_lazy as _ @@ -369,9 +368,9 @@ def get_member_actor_ids(self): # TODO(hybrid-cloud): actor refactor. Remove this method when done. def write_relocation_import( - self, pk_map: PrimaryKeyMap, obj: DeserializedObject, scope: ImportScope + self, pk_map: PrimaryKeyMap, scope: ImportScope ) -> Optional[Tuple[int, int]]: - written = super().write_relocation_import(pk_map, obj, scope) + written = super().write_relocation_import(pk_map, scope) if written is not None: (_, new_pk) = written diff --git a/src/sentry/models/useremail.py b/src/sentry/models/useremail.py index d85d05cfa32989..e398beab8a249f 100644 --- a/src/sentry/models/useremail.py +++ b/src/sentry/models/useremail.py @@ -5,7 +5,6 @@ from typing import TYPE_CHECKING, Iterable, Mapping, Optional, Tuple from django.conf import settings -from django.core.serializers.base import DeserializedObject from django.db import models from django.forms import model_to_dict from django.utils import timezone @@ -88,7 +87,7 @@ def get_primary_email(cls, user: User) -> UserEmail: # with `sentry.user` simultaneously? Will need to make more robust user handling logic, and to # test what happens when a UserEmail already exists. def write_relocation_import( - self, pk_map: PrimaryKeyMap, obj: DeserializedObject, scope: ImportScope + self, pk_map: PrimaryKeyMap, scope: ImportScope ) -> Optional[Tuple[int, int]]: old_pk = super()._normalize_before_relocation_import(pk_map, scope) if old_pk is None: diff --git a/src/sentry/testutils/fixtures.py b/src/sentry/testutils/fixtures.py index a6e0ed2108c478..e5723946d2be74 100644 --- a/src/sentry/testutils/fixtures.py +++ b/src/sentry/testutils/fixtures.py @@ -140,7 +140,8 @@ def create_environment(self, project=None, **kwargs): return Factories.create_environment(project=project, **kwargs) def create_project(self, **kwargs): - kwargs.setdefault("teams", [self.team]) + if "teams" not in kwargs: + kwargs["teams"] = [self.team] return Factories.create_project(**kwargs) def create_project_bookmark(self, project=None, *args, **kwargs): diff --git a/src/sentry/testutils/helpers/backups.py b/src/sentry/testutils/helpers/backups.py index 41988b3c101e9f..666e7db5b32cce 100644 --- a/src/sentry/testutils/helpers/backups.py +++ b/src/sentry/testutils/helpers/backups.py @@ -46,7 +46,6 @@ DashboardWidgetQuery, DashboardWidgetTypes, ) -from sentry.models.environment import EnvironmentProject from sentry.models.integrations.integration import Integration from sentry.models.integrations.organization_integration import OrganizationIntegration from sentry.models.integrations.project_integration import ProjectIntegration @@ -133,8 +132,8 @@ def clear_database(*, reset_pks: bool = False): with transaction.atomic(using="default"): reversed = reversed_dependencies() for model in reversed: - # For some reason, the tables for `SentryApp*` models don't get deleted properly here - # when using `model.objects.all().delete()`, so we have to call out to Postgres + # For some reason, the tables for `SentryApp*` models don't get deleted properly + # here when using `model.objects.all().delete()`, so we have to call out to Postgres # manually. connection = connections[router.db_for_write(SentryApp)] with connection.cursor() as cursor: @@ -285,21 +284,20 @@ def create_exhaustive_organization( }, ) + # Team + team = self.create_team(name=f"test_team_in_{slug}", organization=org) + self.create_team_membership(user=owner, team=team) + OrganizationAccessRequest.objects.create(member=invited, team=team) + # Project* - project = self.create_project() + project = self.create_project(name=f"project-{slug}", teams=[team]) self.create_project_key(project) self.create_project_bookmark(project=project, user=owner) - project = self.create_project() ProjectOwnership.objects.create( project=project, raw='{"hello":"hello"}', schema={"hello": "hello"} ) ProjectRedirect.record(project, f"project_slug_in_{slug}") - # Team - team = self.create_team(name=f"test_team_in_{slug}", organization=org) - self.create_team_membership(user=owner, team=team) - OrganizationAccessRequest.objects.create(member=invited, team=team) - # Integration* integration = Integration.objects.create( provider="slack", name=f"Slack for {slug}", external_id=f"slack:{slug}" @@ -318,8 +316,7 @@ def create_exhaustive_organization( self.snooze_rule(user_id=owner_id, owner_id=owner_id, rule=rule) # Environment* - env = self.create_environment() - EnvironmentProject.objects.create(project=project, environment=env, is_hidden=False) + self.create_environment(project=project) # Monitor Monitor.objects.create( @@ -330,12 +327,18 @@ def create_exhaustive_organization( ) # AlertRule* - alert = self.create_alert_rule(include_all_projects=True, excluded_projects=[project]) - trigger = self.create_alert_rule_trigger(alert_rule=alert, excluded_projects=[self.project]) + other_project = self.create_project(name=f"other-project-{slug}", teams=[team]) + alert = self.create_alert_rule( + organization=org, + projects=[project], + include_all_projects=True, + excluded_projects=[other_project], + ) + trigger = self.create_alert_rule_trigger(alert_rule=alert, excluded_projects=[project]) self.create_alert_rule_trigger_action(alert_rule_trigger=trigger) # Incident* - incident = self.create_incident() + incident = self.create_incident(org, [project]) IncidentActivity.objects.create( incident=incident, type=1, @@ -395,7 +398,7 @@ def create_exhaustive_organization( # misc Counter.increment(project, 1) self.create_repo( - project=self.project, + project=project, name="getsentry/getsentry", provider="integrations:github", integration_id=integration.id, diff --git a/tests/sentry/backup/test_imports.py b/tests/sentry/backup/test_imports.py index b58fc72c674e51..3e733cad109402 100644 --- a/tests/sentry/backup/test_imports.py +++ b/tests/sentry/backup/test_imports.py @@ -17,8 +17,14 @@ from sentry.backup.scopes import ExportScope, RelocationScope from sentry.models.authenticator import Authenticator from sentry.models.email import Email +from sentry.models.options.project_option import ProjectOption from sentry.models.organization import Organization +from sentry.models.organizationmapping import OrganizationMapping +from sentry.models.organizationmember import OrganizationMember +from sentry.models.organizationmembermapping import OrganizationMemberMapping from sentry.models.orgauthtoken import OrgAuthToken +from sentry.models.project import Project +from sentry.models.projectkey import ProjectKey from sentry.models.user import User from sentry.models.useremail import UserEmail from sentry.models.userip import UserIP @@ -163,6 +169,61 @@ def export_to_tmp_file_and_clear_database(self, tmp_dir) -> Path: return tmp_path +@run_backup_tests_only_on_single_db +class SignalingTests(ImportTestCase): + """ + Some models are automatically created via signals and similar automagic from related models. We test that behavior here. + """ + + def test_import_signaling_user(self): + self.create_exhaustive_user("user", email="me@example.com") + + with tempfile.TemporaryDirectory() as tmp_dir: + tmp_path = self.export_to_tmp_file_and_clear_database(tmp_dir) + with open(tmp_path) as tmp_file: + import_in_user_scope(tmp_file, printer=NOOP_PRINTER) + + assert User.objects.count() == 1 + assert User.objects.filter(email="me@example.com").exists() + + assert UserEmail.objects.count() == 1 + assert UserEmail.objects.filter(email="me@example.com").exists() + + assert Email.objects.count() == 1 + assert Email.objects.filter(email="me@example.com").exists() + + def test_import_signaling_organization(self): + owner = self.create_exhaustive_user("owner") + invited = self.create_exhaustive_user("invited") + member = self.create_exhaustive_user("member") + self.create_exhaustive_organization("some-org", owner, invited, [member]) + + with tempfile.TemporaryDirectory() as tmp_dir: + tmp_path = self.export_to_tmp_file_and_clear_database(tmp_dir) + with open(tmp_path) as tmp_file: + import_in_organization_scope(tmp_file, printer=NOOP_PRINTER) + + assert Organization.objects.count() == 1 + assert Organization.objects.filter(slug="some-org").exists() + + assert OrganizationMapping.objects.count() == 1 + assert OrganizationMapping.objects.filter(slug="some-org").exists() + + assert OrganizationMember.objects.count() == 3 + assert OrganizationMemberMapping.objects.count() == 3 + + # The exhaustive org has 2 projects which automatically get 1 key and 3 options each. + assert Project.objects.count() == 2 + assert Project.objects.filter(name="project-some-org").exists() + assert Project.objects.filter(name="other-project-some-org").exists() + + assert ProjectKey.objects.count() == 2 + assert ProjectOption.objects.count() == 6 + assert ProjectOption.objects.filter(key="sentry:relay-rev").exists() + assert ProjectOption.objects.filter(key="sentry:relay-rev-lastchange").exists() + assert ProjectOption.objects.filter(key="sentry:option-epoch").exists() + + @run_backup_tests_only_on_single_db class ScopingTests(ImportTestCase): """ diff --git a/tests/sentry/backup/test_roundtrip.py b/tests/sentry/backup/test_roundtrip.py index 779845660d64e5..ab5f03487497b3 100644 --- a/tests/sentry/backup/test_roundtrip.py +++ b/tests/sentry/backup/test_roundtrip.py @@ -30,19 +30,10 @@ def test_bad_unequal_json(tmp_path): import_export_from_fixture_then_validate(tmp_path, "fresh-install.json") findings = execinfo.value.info.findings - assert len(findings) == 3 + assert len(findings) >= 3 assert findings[0].kind == ComparatorFindingKind.UnequalJSON - assert findings[0].on == InstanceID("sentry.useremail", 1) - assert findings[0].left_pk == 1 - assert findings[0].right_pk == 1 assert findings[1].kind == ComparatorFindingKind.UnequalJSON - assert findings[1].on == InstanceID("sentry.userrole", 1) - assert findings[1].left_pk == 1 - assert findings[1].right_pk == 1 assert findings[2].kind == ComparatorFindingKind.UnequalJSON - assert findings[2].on == InstanceID("sentry.userroleuser", 1) - assert findings[2].left_pk == 1 - assert findings[2].right_pk == 1 @run_backup_tests_only_on_single_db