Skip to content

Commit 1d43a7a

Browse files
committed
perf(api): Omit full resources from control organization contexts
Default control organization endpoints to lightweight RPC contexts, with independent opt-ins for endpoints that need projects or teams. Cover both organization route forms and preserve member access. Refs ISWF-3361
1 parent 66dffbb commit 1d43a7a

2 files changed

Lines changed: 61 additions & 58 deletions

File tree

‎src/sentry/api/bases/organization.py‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -277,10 +277,10 @@ class ControlSiloOrganizationEndpoint(Endpoint):
277277

278278
# Control silo endpoints fetch the organization over RPC from the cell silo.
279279
# Including projects/teams serializes every row into the response body, which is
280-
# expensive for large organizations. Set these to False when the endpoint does not
281-
# need them.
282-
include_organization_projects: bool = True
283-
include_organization_teams: bool = True
280+
# expensive for large organizations. Endpoints that consume these collections must
281+
# explicitly opt in; permission checks do not require them.
282+
include_organization_projects: bool = False
283+
include_organization_teams: bool = False
284284

285285
def convert_args(
286286
self,

‎tests/sentry/api/bases/test_organization.py‎

Lines changed: 57 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -61,9 +61,9 @@ def is_active(self) -> bool:
6161
return True
6262

6363

64-
class LightweightControlSiloOrganizationEndpoint(ControlSiloOrganizationEndpoint):
65-
include_organization_projects = False
66-
include_organization_teams = False
64+
class HydratedControlSiloOrganizationEndpoint(ControlSiloOrganizationEndpoint):
65+
include_organization_projects = True
66+
include_organization_teams = True
6767

6868

6969
class PermissionBaseTestCase(TestCase):
@@ -481,11 +481,14 @@ def test_existing_context_is_reused(self) -> None:
481481
assert request.access.has_scope("org:read")
482482

483483

484+
@all_silo_test
484485
class ControlSiloOrganizationEndpointTest(TestCase):
485486
def setUp(self) -> None:
486487
super().setUp()
487488
self.user = self.create_user()
488489
self.organization = self.create_organization(owner=self.user)
490+
self.team = self.create_team(organization=self.organization)
491+
self.project = self.create_project(organization=self.organization, teams=[self.team])
489492

490493
def build_request(self):
491494
request = RequestFactory().get("/")
@@ -494,62 +497,62 @@ def build_request(self):
494497
request.auth = None
495498
return drf_request_from_request(request)
496499

497-
@mock.patch.object(
498-
organization_service,
499-
"get_organization_by_slug",
500-
wraps=organization_service.get_organization_by_slug,
501-
)
502-
def test_convert_args_includes_projects_and_teams_by_default(
503-
self, mock_get_organization: mock.MagicMock
504-
) -> None:
505-
ControlSiloOrganizationEndpoint().convert_args(self.build_request(), self.organization.slug)
506-
507-
mock_get_organization.assert_called_once_with(
508-
slug=self.organization.slug,
509-
only_visible=False,
510-
user_id=self.user.id,
511-
include_projects=True,
512-
include_teams=True,
513-
)
500+
def assert_lightweight_context(self, organization_id_or_slug: int | str) -> None:
501+
request = self.build_request()
502+
with (
503+
mock.patch(
504+
"sentry.organizations.services.organization.serial.serialize_project"
505+
) as serialize_project,
506+
mock.patch(
507+
"sentry.organizations.services.organization.serial.serialize_rpc_team"
508+
) as serialize_team,
509+
):
510+
_, kwargs = ControlSiloOrganizationEndpoint().convert_args(
511+
request, organization_id_or_slug
512+
)
514513

515-
@mock.patch.object(
516-
organization_service,
517-
"get_organization_by_slug",
518-
wraps=organization_service.get_organization_by_slug,
519-
)
520-
def test_convert_args_can_omit_projects_and_teams_for_slug(
521-
self, mock_get_organization: mock.MagicMock
522-
) -> None:
523-
LightweightControlSiloOrganizationEndpoint().convert_args(
524-
self.build_request(), self.organization.slug
525-
)
514+
serialize_project.assert_not_called()
515+
serialize_team.assert_not_called()
516+
assert kwargs["organization"].projects == []
517+
assert kwargs["organization"].teams == []
518+
assert request.access.has_scope("org:read")
519+
assert request.access.has_project_access(self.project)
520+
assert request.access.has_team_access(self.team)
526521

527-
mock_get_organization.assert_called_once_with(
528-
slug=self.organization.slug,
529-
only_visible=False,
530-
user_id=self.user.id,
531-
include_projects=False,
532-
include_teams=False,
533-
)
522+
def test_convert_args_omits_projects_and_teams_by_default_for_slug(self) -> None:
523+
self.assert_lightweight_context(self.organization.slug)
534524

535-
@mock.patch.object(
536-
organization_service,
537-
"get_organization_by_id",
538-
wraps=organization_service.get_organization_by_id,
539-
)
540-
def test_convert_args_can_omit_projects_and_teams_for_id(
541-
self, mock_get_organization: mock.MagicMock
542-
) -> None:
543-
LightweightControlSiloOrganizationEndpoint().convert_args(
544-
self.build_request(), self.organization.id
545-
)
525+
def test_convert_args_omits_projects_and_teams_by_default_for_id(self) -> None:
526+
self.assert_lightweight_context(self.organization.id)
546527

547-
mock_get_organization.assert_called_once_with(
548-
id=self.organization.id,
549-
user_id=self.user.id,
550-
include_projects=False,
551-
include_teams=False,
528+
def assert_hydrated_context(self, organization_id_or_slug: int | str) -> None:
529+
_, kwargs = HydratedControlSiloOrganizationEndpoint().convert_args(
530+
self.build_request(), organization_id_or_slug
552531
)
532+
assert {p.id for p in kwargs["organization"].projects} == {self.project.id}
533+
assert {t.id for t in kwargs["organization"].teams} == {self.team.id}
534+
535+
def test_convert_args_can_include_projects_and_teams_for_slug(self) -> None:
536+
self.assert_hydrated_context(self.organization.slug)
537+
538+
def test_convert_args_can_include_projects_and_teams_for_id(self) -> None:
539+
self.assert_hydrated_context(self.organization.id)
540+
541+
def test_convert_args_can_include_only_projects(self) -> None:
542+
endpoint = ControlSiloOrganizationEndpoint()
543+
endpoint.include_organization_projects = True
544+
_, kwargs = endpoint.convert_args(self.build_request(), self.organization.slug)
545+
546+
assert {p.id for p in kwargs["organization"].projects} == {self.project.id}
547+
assert kwargs["organization"].teams == []
548+
549+
def test_convert_args_can_include_only_teams(self) -> None:
550+
endpoint = ControlSiloOrganizationEndpoint()
551+
endpoint.include_organization_teams = True
552+
_, kwargs = endpoint.convert_args(self.build_request(), self.organization.id)
553+
554+
assert kwargs["organization"].projects == []
555+
assert {t.id for t in kwargs["organization"].teams} == {self.team.id}
553556

554557

555558
class OrganizationEndpointViewerContextTest(BaseOrganizationEndpointTest):

0 commit comments

Comments
 (0)