ref(hc): Common Option interface/mixin for ORM and RPC models (#50335)

Extract from `Organization` and `Project` a mixin for common methods
related to options, and have `RpcOrganization` and `RpcProject`
implement the same interface.

Introduce `ProjectService`. Add RPC methods for reading, writing, and
deleting organization and project options.

pyproject.toml

@@ -928,7 +928,6 @@ module = [
     "sentry.replays.testutils",
     "sentry.replays.usecases.ingest.dom_index",
     "sentry.replays.usecases.reader",
-    "sentry.reprocessing",
     "sentry.reprocessing2",
     "sentry.roles",
     "sentry.rules.actions.integrations.base",
@@ -1335,7 +1334,6 @@ module = [
     "tests.sentry.api.endpoints.test_project_dynamic_sampling",
     "tests.sentry.api.endpoints.test_project_group_stats",
     "tests.sentry.api.endpoints.test_project_ownership",
-    "tests.sentry.api.endpoints.test_project_plugin_details",
     "tests.sentry.api.endpoints.test_project_release_details",
     "tests.sentry.api.endpoints.test_project_release_file_details",
     "tests.sentry.api.endpoints.test_project_releases",
@@ -1616,7 +1614,6 @@ module = [
     "tests.sentry.models.test_organizationmember",
     "tests.sentry.models.test_organizationoption",
     "tests.sentry.models.test_project",
-    "tests.sentry.models.test_projectoption",
     "tests.sentry.models.test_projectownership",
     "tests.sentry.models.test_release",
     "tests.sentry.models.test_releasefile",

src/sentry/models/options/option.py

@@ -1,7 +1,19 @@
+from __future__ import annotations
+
+import abc
+
 from django.db import models
 from django.utils import timezone
 
-from sentry.db.models import Model, control_silo_only_model, region_silo_only_model, sane_repr
+from sentry.db.models import (
+    Model,
+    OptionManager,
+    ValidateFunction,
+    Value,
+    control_silo_only_model,
+    region_silo_only_model,
+    sane_repr,
+)
 from sentry.db.models.fields.picklefield import PickledObjectField
 from sentry.options.manager import UpdateChannel
 
@@ -51,3 +63,43 @@ class ControlOption(BaseOption):
         db_table = "sentry_controloption"
 
     __repr__ = sane_repr("key", "value")
+
+
+class HasOption:
+    # Logically this is an abstract interface. Leaving off abc.ABC because it clashes
+    # with the Model metaclass.
+
+    @abc.abstractmethod
+    def get_option(
+        self,
+        key: str,
+        default: Value | None = None,
+        validate: ValidateFunction | None = None,
+    ) -> Value:
+        raise NotImplementedError
+
+    @abc.abstractmethod
+    def update_option(self, key: str, value: Value) -> bool:
+        raise NotImplementedError
+
+    @abc.abstractmethod
+    def delete_option(self, key: str) -> None:
+        raise NotImplementedError
+
+
+class OptionMixin(HasOption):
+    @property
+    @abc.abstractmethod
+    def option_manager(self) -> OptionManager:
+        raise NotImplementedError
+
+    def get_option(
+        self, key: str, default: Value | None = None, validate: ValidateFunction | None = None
+    ) -> Value:
+        return self.option_manager.get_value(self, key, default, validate)
+
+    def update_option(self, key: str, value: Value) -> bool:
+        return self.option_manager.set_value(self, key, value)
+
+    def delete_option(self, key: str) -> None:
+        self.option_manager.unset_value(self, key)

src/sentry/models/options/organization_option.py

@@ -6,8 +6,7 @@ from django.db import models
 
 from sentry.db.models import FlexibleForeignKey, Model, region_silo_only_model, sane_repr
 from sentry.db.models.fields.picklefield import PickledObjectField
-from sentry.db.models.manager import OptionManager, Value
-from sentry.tasks.relay import schedule_invalidate_project_config
+from sentry.db.models.manager import OptionManager, ValidateFunction, Value
 from sentry.utils.cache import cache
 
 if TYPE_CHECKING:
@@ -26,7 +25,11 @@ class OrganizationOptionManager(OptionManager["Organization"]):
         return result
 
     def get_value(
-        self, organization: Organization, key: str, default: Value | None = None
+        self,
+        organization: Organization,
+        key: str,
+        default: Value | None = None,
+        validate: ValidateFunction | None = None,
     ) -> Value:
         result = self.get_all_values(organization)
         return result.get(key, default)
@@ -39,9 +42,12 @@ class OrganizationOptionManager(OptionManager["Organization"]):
         inst.delete()
         self.reload_cache(organization.id, "organizationoption.unset_value")
 
-    def set_value(self, organization: Organization, key: str, value: Value) -> None:
-        self.create_or_update(organization=organization, key=key, values={"value": value})
+    def set_value(self, organization: Organization, key: str, value: Value) -> bool:
+        inst, created = self.create_or_update(
+            organization=organization, key=key, values={"value": value}
+        )
         self.reload_cache(organization.id, "organizationoption.set_value")
+        return bool(created) or inst > 0
 
     def get_all_values(self, organization: Organization) -> Mapping[str, Value]:
         if isinstance(organization, models.Model):
@@ -62,6 +68,8 @@ class OrganizationOptionManager(OptionManager["Organization"]):
         return values
 
     def reload_cache(self, organization_id: int, update_reason: str) -> Mapping[str, Value]:
+        from sentry.tasks.relay import schedule_invalidate_project_config
+
         if update_reason != "organizationoption.get_all_values":
             schedule_invalidate_project_config(
                 organization_id=organization_id, trigger=update_reason

src/sentry/models/options/project_option.py

@@ -8,7 +8,6 @@ from sentry import projectoptions
 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
-from sentry.tasks.relay import schedule_invalidate_project_config
 from sentry.utils.cache import cache
 
 if TYPE_CHECKING:
@@ -121,6 +120,8 @@ class ProjectOptionManager(OptionManager["Project"]):
         return values
 
     def reload_cache(self, project_id: int, update_reason: str) -> Mapping[str, Value]:
+        from sentry.tasks.relay import schedule_invalidate_project_config
+
         if update_reason != "projectoption.get_all_values":
             schedule_invalidate_project_config(project_id=project_id, trigger=update_reason)
         cache_key = self._make_key(project_id)

src/sentry/models/organization.py

@@ -25,12 +25,14 @@ from sentry.db.models import (
     BaseManager,
     BoundedPositiveIntegerField,
     Model,
+    OptionManager,
     region_silo_only_model,
     sane_repr,
 )
 from sentry.db.models.utils import slugify_instance
 from sentry.db.postgres.roles import in_test_psql_role_override
 from sentry.locks import locks
+from sentry.models.options.option import OptionMixin
 from sentry.models.organizationmember import OrganizationMember
 from sentry.models.organizationmemberteam import OrganizationMemberTeam
 from sentry.models.outbox import OutboxCategory, OutboxScope, RegionOutbox
@@ -158,7 +160,7 @@ class OrganizationManager(BaseManager):
 
 
 @region_silo_only_model
-class Organization(Model, OrganizationAbsoluteUrlMixin, SnowflakeIdMixin):
+class Organization(Model, OptionMixin, OrganizationAbsoluteUrlMixin, SnowflakeIdMixin):
     """
     An organization represents a group of individuals which maintain ownership of projects.
     """
@@ -545,21 +547,11 @@ class Organization(Model, OrganizationAbsoluteUrlMixin, SnowflakeIdMixin):
             queryset = model.objects.filter(organization_id=from_org.id)
             do_update(queryset, {"organization_id": to_org.id})
 
-    # TODO: Make these a mixin
-    def update_option(self, *args, **kwargs):
-        from sentry.models import OrganizationOption
-
-        return OrganizationOption.objects.set_value(self, *args, **kwargs)
-
-    def get_option(self, *args, **kwargs):
-        from sentry.models import OrganizationOption
-
-        return OrganizationOption.objects.get_value(self, *args, **kwargs)
-
-    def delete_option(self, *args, **kwargs):
+    @property
+    def option_manager(self) -> OptionManager:
         from sentry.models import OrganizationOption
 
-        return OrganizationOption.objects.unset_value(self, *args, **kwargs)
+        return OrganizationOption.objects
 
     def send_delete_confirmation(self, audit_log_entry, countdown):
         from sentry import options

src/sentry/models/project.py

@@ -25,12 +25,15 @@ from sentry.db.models import (
     BoundedPositiveIntegerField,
     FlexibleForeignKey,
     Model,
+    OptionManager,
+    Value,
     region_silo_only_model,
     sane_repr,
 )
 from sentry.db.models.utils import slugify_instance
 from sentry.db.postgres.roles import in_test_psql_role_override
 from sentry.locks import locks
+from sentry.models import OptionMixin
 from sentry.models.outbox import OutboxCategory, OutboxScope, RegionOutbox
 from sentry.services.hybrid_cloud.user import RpcUser
 from sentry.services.hybrid_cloud.user.service import user_service
@@ -103,7 +106,7 @@ class ProjectManager(BaseManager):
 
 
 @region_silo_only_model
-class Project(Model, PendingDeletionMixin, SnowflakeIdMixin):
+class Project(Model, PendingDeletionMixin, OptionMixin, SnowflakeIdMixin):
     from sentry.models.projectteam import ProjectTeam
 
     """
@@ -222,15 +225,19 @@ class Project(Model, PendingDeletionMixin, SnowflakeIdMixin):
                 return True
         return False
 
-    # TODO: Make these a mixin
-    def update_option(self, *args, **kwargs):
-        return projectoptions.set(self, *args, **kwargs)
+    @property
+    def option_manager(self) -> OptionManager:
+        from sentry.models import ProjectOption
+
+        return ProjectOption.objects
 
-    def get_option(self, *args, **kwargs):
-        return projectoptions.get(self, *args, **kwargs)
+    def update_option(self, key: str, value: Value) -> bool:
+        projectoptions.update_rev_for_option(self)
+        return super().update_option(key, value)
 
-    def delete_option(self, *args, **kwargs):
-        return projectoptions.delete(self, *args, **kwargs)
+    def delete_option(self, key: str) -> None:
+        projectoptions.update_rev_for_option(self)
+        super().delete_option(key)
 
     def update_rev_for_option(self):
         return projectoptions.update_rev_for_option(self)

src/sentry/services/hybrid_cloud/__init__.py

@@ -35,6 +35,8 @@ T = TypeVar("T")
 
 ArgumentDict = Mapping[str, Any]
 
+OptionValue = Union[str, int, bool, None]
+
 IDEMPOTENCY_KEY_LENGTH = 48
 REGION_NAME_LENGTH = 48
 

src/sentry/services/hybrid_cloud/organization/impl.py

@@ -20,7 +20,7 @@ from sentry.models import (
     Team,
 )
 from sentry.models.organizationmember import InviteStatus
-from sentry.services.hybrid_cloud import logger
+from sentry.services.hybrid_cloud import OptionValue, logger
 from sentry.services.hybrid_cloud.organization import (
     OrganizationService,
     RpcOrganizationInvite,
@@ -466,3 +466,18 @@ class DatabaseBackedOrganizationService(OrganizationService):
             OrganizationMember.objects.filter(user_id=user.id).update(
                 user_is_active=user.is_active, user_email=user.email
             )
+
+    def get_option(self, *, organization_id: int, key: str) -> OptionValue:
+        orm_organization = Organization.objects.get_from_cache(id=organization_id)
+        value = orm_organization.get_option(key)
+        if value is not None and not isinstance(value, (str, int, bool)):
+            raise TypeError
+        return value
+
+    def update_option(self, *, organization_id: int, key: str, value: OptionValue) -> bool:
+        orm_organization = Organization.objects.get_from_cache(id=organization_id)
+        return orm_organization.update_option(key, value)  # type: ignore[no-any-return]
+
+    def delete_option(self, *, organization_id: int, key: str) -> None:
+        orm_organization = Organization.objects.get_from_cache(id=organization_id)
+        orm_organization.delete_option(key)

src/sentry/services/hybrid_cloud/organization/model.py

@@ -8,9 +8,12 @@ from typing import Any, List, Mapping, Optional
 from pydantic import Field
 
 from sentry.constants import ObjectStatus
+from sentry.db.models import ValidateFunction, Value
+from sentry.models.options.option import HasOption
 from sentry.roles import team_roles
 from sentry.roles.manager import TeamRole
 from sentry.services.hybrid_cloud import RpcModel
+from sentry.services.hybrid_cloud.project import RpcProject
 from sentry.types.organization import OrganizationAbsoluteUrlMixin
 
 
@@ -66,14 +69,6 @@ def project_status_visible() -> int:
     return int(ObjectStatus.ACTIVE)
 
 
-class RpcProject(RpcModel):
-    id: int = -1
-    slug: str = ""
-    name: str = ""
-    organization_id: int = -1
-    status: int = Field(default_factory=project_status_visible)
-
-
 class RpcOrganizationMemberFlags(RpcModel):
     sso__linked: bool = False
     sso__invalid: bool = False
@@ -152,7 +147,7 @@ class RpcOrganizationInvite(RpcModel):
     email: str = ""
 
 
-class RpcOrganizationSummary(RpcModel, OrganizationAbsoluteUrlMixin):
+class RpcOrganizationSummary(RpcModel, OrganizationAbsoluteUrlMixin, HasOption):
     """
     The subset of organization metadata available from the control silo specifically.
     """
@@ -166,6 +161,23 @@ class RpcOrganizationSummary(RpcModel, OrganizationAbsoluteUrlMixin):
         # serializers, as this organization summary object is often used for that.
         return hash((self.id, self.slug))
 
+    def get_option(
+        self, key: str, default: Optional[Value] = None, validate: Optional[ValidateFunction] = None
+    ) -> Value:
+        from sentry.services.hybrid_cloud.organization import organization_service
+
+        return organization_service.get_option(organization_id=self.id, key=key)
+
+    def update_option(self, key: str, value: Value) -> bool:
+        from sentry.services.hybrid_cloud.organization import organization_service
+
+        return organization_service.update_option(organization_id=self.id, key=key, value=value)
+
+    def delete_option(self, key: str) -> None:
+        from sentry.services.hybrid_cloud.organization import organization_service
+
+        organization_service.delete_option(organization_id=self.id, key=key)
+
 
 class RpcOrganization(RpcOrganizationSummary):
     # Represents the full set of teams and projects associated with the org.  Note that these are not filtered by

src/sentry/services/hybrid_cloud/organization/serial.py

@@ -20,10 +20,10 @@ from sentry.services.hybrid_cloud.organization import (
     RpcOrganizationMemberFlags,
     RpcOrganizationMemberSummary,
     RpcOrganizationSummary,
-    RpcProject,
     RpcTeam,
     RpcTeamMember,
 )
+from sentry.services.hybrid_cloud.project.serial import serialize_project
 
 
 def escape_flag_name(flag_name: str) -> str:
@@ -125,16 +125,6 @@ def _serialize_team_member(
     return result
 
 
-def _serialize_project(project: Project) -> RpcProject:
-    return RpcProject(
-        id=project.id,
-        slug=project.slug,
-        name=project.name,
-        organization_id=project.organization_id,
-        status=project.status,
-    )
-
-
 def serialize_organization_summary(org: Organization) -> RpcOrganizationSummary:
     return RpcOrganizationSummary(
         slug=org.slug,
@@ -155,6 +145,6 @@ def serialize_organization(org: Organization) -> RpcOrganization:
 
     projects: List[Project] = Project.objects.filter(organization=org)
     teams: List[Team] = Team.objects.filter(organization=org)
-    rpc_org.projects.extend(_serialize_project(project) for project in projects)
+    rpc_org.projects.extend(serialize_project(project) for project in projects)
     rpc_org.teams.extend(_serialize_team(team) for team in teams)
     return rpc_org