From 0d52a82714f186cd268a849f0244738517cb0339 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 21 Apr 2026 14:48:30 -0400 Subject: [PATCH 1/4] Add an OrganizationProject purge key for the org profile Signed-off-by: William Woodruff --- tests/unit/cache/origin/test_init.py | 52 ++++++++++++++++++++++++++++ tests/unit/packaging/test_init.py | 3 ++ warehouse/organizations/services.py | 14 ++++---- warehouse/packaging/__init__.py | 1 + warehouse/packaging/services.py | 11 ++++-- 5 files changed, 72 insertions(+), 9 deletions(-) diff --git a/tests/unit/cache/origin/test_init.py b/tests/unit/cache/origin/test_init.py index e4ecfdfa5140..1c4771d70763 100644 --- a/tests/unit/cache/origin/test_init.py +++ b/tests/unit/cache/origin/test_init.py @@ -4,6 +4,10 @@ import pytest from tests.common.db.accounts import UserFactory +from tests.common.db.organizations import ( + OrganizationFactory, + OrganizationProjectFactory, +) from tests.common.db.packaging import ( FileFactory, ProjectFactory, @@ -16,6 +20,7 @@ from warehouse.cache.origin.derivers import html_cache_deriver from warehouse.cache.origin.interfaces import IOriginCache from warehouse.observations.models import ObservationKind +from warehouse.organizations.services import database_organization_factory def test_store_purge_keys(): @@ -113,6 +118,53 @@ def test_store_purge_keys_skips_audit_only_collection_changes( assert f"project/{project.normalized_name}" not in purges +def test_store_purge_keys_organization_project_add_purges_org_and_project( + app_config, db_request +): + # Adding a project to an organization via `add_organization_project` must + # purge both the project profile (`project/{name}`) and the organization + # profile (`org/{name}`), otherwise the cached `/org/{orgname}` project + # list goes stale. + # + # Exercising `add_organization_project is load-bearing: the + # purge-key factory uses `if_attr_exists`, which resolves to `None` during + # `after_flush` if the row was built with only FK ids instead of + # objects, causing us to silently drop the purges. + organization = OrganizationFactory.create() + project = ProjectFactory.create() + db_request.db.flush() + db_request.db.info.pop("warehouse.cache.origin.purges", None) + + organization_service = database_organization_factory(None, db_request) + organization_service.add_organization_project(organization.id, project.id) + db_request.db.flush() + + purges = db_request.db.info.get("warehouse.cache.origin.purges", set()) + assert f"org/{organization.normalized_name}" in purges + assert f"project/{project.normalized_name}" in purges + + +def test_store_purge_keys_organization_project_delete_purges_org_and_project( + app_config, db_request +): + # Deleting a project from an organization via `delete_organization_project` + # must also purge both caches so the org profile reflects the project + # leaving the organization. + organization = OrganizationFactory.create() + project = ProjectFactory.create() + OrganizationProjectFactory.create(organization=organization, project=project) + db_request.db.flush() + db_request.db.info.pop("warehouse.cache.origin.purges", None) + + organization_service = database_organization_factory(None, db_request) + organization_service.delete_organization_project(organization.id, project.id) + db_request.db.flush() + + purges = db_request.db.info.get("warehouse.cache.origin.purges", set()) + assert f"org/{organization.normalized_name}" in purges + assert f"project/{project.normalized_name}" in purges + + def test_store_purge_keys_skips_project_dirty_on_roles_change(app_config, db_request): # Role has its own cache_keys; the Project-dirty purge would only add # all-projects and org/{name}, which aren't affected by role membership. diff --git a/tests/unit/packaging/test_init.py b/tests/unit/packaging/test_init.py index 15f77e1baee9..bb50a55d8031 100644 --- a/tests/unit/packaging/test_init.py +++ b/tests/unit/packaging/test_init.py @@ -156,6 +156,9 @@ def key_factory(keystring, iterate_on=None, if_attr_exists=None): OrganizationProject, purge_keys=[ key_factory("project/{attr.normalized_name}", if_attr_exists="project"), + key_factory( + "org/{attr.normalized_name}", if_attr_exists="organization" + ), ], ), ] diff --git a/warehouse/organizations/services.py b/warehouse/organizations/services.py index 33cef02e0601..116a0218db4e 100644 --- a/warehouse/organizations/services.py +++ b/warehouse/organizations/services.py @@ -3,7 +3,7 @@ import datetime from psycopg.errors import UniqueViolation -from sqlalchemy import delete, func, orm, select +from sqlalchemy import delete, func, select from sqlalchemy.exc import NoResultFound from zope.interface import implementer @@ -556,16 +556,16 @@ def add_organization_project(self, organization_id, project_id): """ Adds an association between the specified organization and project """ + from warehouse.packaging.models import Project + + # We need to pass the full objects instead of just their IDs + # to avoid silently breaking our purges that rely on `if_attr_exists`. organization_project = OrganizationProject( - organization_id=organization_id, - project_id=project_id, + organization=self.db.get(Organization, organization_id), + project=self.db.get(Project, project_id), ) self.db.add(organization_project) - self.db.flush() # Flush db so we can address the organization related object - - # Mark Organization as dirty, so purges will happen - orm.attributes.flag_dirty(organization_project.organization) return organization_project diff --git a/warehouse/packaging/__init__.py b/warehouse/packaging/__init__.py index d2e97723a6eb..961e54901a57 100644 --- a/warehouse/packaging/__init__.py +++ b/warehouse/packaging/__init__.py @@ -173,6 +173,7 @@ def includeme(config): OrganizationProject, purge_keys=[ key_factory("project/{attr.normalized_name}", if_attr_exists="project"), + key_factory("org/{attr.normalized_name}", if_attr_exists="organization"), ], ) diff --git a/warehouse/packaging/services.py b/warehouse/packaging/services.py index cd685c18dfb3..2e2432874859 100644 --- a/warehouse/packaging/services.py +++ b/warehouse/packaging/services.py @@ -667,10 +667,17 @@ def create_project( ) if organization_id: - # If an organization ID is provided, we never set the creator to owner + # If an organization ID is provided, we never set the creator to owner. + # Pass the resolved `organization` object (not just `organization_id`) + # so the `OrganizationProject` purge-key factory can resolve the + # relationship during `after_flush` and actually emit the + # `org/{name}` and `project/{name}` purge keys. + from warehouse.organizations.models import Organization + self.db.add( OrganizationProject( - organization_id=organization_id, project_id=project.id + organization=self.db.get(Organization, organization_id), + project=project, ) ) elif creator_is_owner: From 32f7aeb7ae5f9ad3a03cc8c02185714ec2192646 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 21 Apr 2026 16:04:42 -0400 Subject: [PATCH 2/4] Add a backstop test Signed-off-by: William Woodruff --- tests/unit/cache/origin/test_init.py | 42 ++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/unit/cache/origin/test_init.py b/tests/unit/cache/origin/test_init.py index 1c4771d70763..634282e0700e 100644 --- a/tests/unit/cache/origin/test_init.py +++ b/tests/unit/cache/origin/test_init.py @@ -144,6 +144,48 @@ def test_store_purge_keys_organization_project_add_purges_org_and_project( assert f"project/{project.normalized_name}" in purges +def test_store_purge_keys_organization_project_add_survives_events_committed_state( + app_config, db_request +): + # Regression guard for issue #19911. + # + # `store_purge_keys` skips emitting a purge for a dirty object whose + # `committed_state` is a subset of `_NON_CACHE_RELEVANT_ATTRS` (the + # perf optimizations in PRs #19898 and #19910). Before this fix, + # `add_organization_project` relied on `flag_dirty(org)` to trigger an + # Organization-dirty purge — but the manage view then recorded an event + # on the organization, setting `event.source = org` and back-populating + # the `events` dynamic collection, which put `'events'` in the + # Organization's committed_state. The skip then masked the only purge + # for `org/{orgname}`, leaving the CDN entry stale. + # + # The fix makes the purge come from the new OrganizationProject row + # (session.new, not session.dirty) — the skip applies only to dirty + # objects, so a `session.new` row is unaffected. This test pins down + # that behavior: the `org/{name}` purge must still fire even when the + # Organization has `events` in its committed_state at flush time. + organization = OrganizationFactory.create() + project = ProjectFactory.create() + db_request.db.flush() + db_request.db.info.pop("warehouse.cache.origin.purges", None) + + organization_service = database_organization_factory(None, db_request) + organization_service.add_organization_project(organization.id, project.id) + + # Construct an OrganizationEvent with `source=organization` to force + # `events` into Organization's committed_state before the next flush. + # This is the exact state the manage view produces via `record_event`. + db_request.db.add( + organization.Event(source=organization, tag="test:regression") + ) + + db_request.db.flush() + + purges = db_request.db.info.get("warehouse.cache.origin.purges", set()) + assert f"org/{organization.normalized_name}" in purges + assert f"project/{project.normalized_name}" in purges + + def test_store_purge_keys_organization_project_delete_purges_org_and_project( app_config, db_request ): From ef94b779434559057f55c131ec36c08f3e0bc0f8 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Tue, 21 Apr 2026 17:42:31 -0400 Subject: [PATCH 3/4] Reformat Signed-off-by: William Woodruff --- tests/unit/cache/origin/test_init.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/unit/cache/origin/test_init.py b/tests/unit/cache/origin/test_init.py index 634282e0700e..fd7bc2c03002 100644 --- a/tests/unit/cache/origin/test_init.py +++ b/tests/unit/cache/origin/test_init.py @@ -175,9 +175,7 @@ def test_store_purge_keys_organization_project_add_survives_events_committed_sta # Construct an OrganizationEvent with `source=organization` to force # `events` into Organization's committed_state before the next flush. # This is the exact state the manage view produces via `record_event`. - db_request.db.add( - organization.Event(source=organization, tag="test:regression") - ) + db_request.db.add(organization.Event(source=organization, tag="test:regression")) db_request.db.flush() From 25bc4041168dc0adf8bae53ee86e805f72aae191 Mon Sep 17 00:00:00 2001 From: William Woodruff Date: Wed, 22 Apr 2026 10:27:30 -0400 Subject: [PATCH 4/4] Trim comment Signed-off-by: William Woodruff --- tests/unit/cache/origin/test_init.py | 17 ++--------------- 1 file changed, 2 insertions(+), 15 deletions(-) diff --git a/tests/unit/cache/origin/test_init.py b/tests/unit/cache/origin/test_init.py index fd7bc2c03002..881a6adf9e41 100644 --- a/tests/unit/cache/origin/test_init.py +++ b/tests/unit/cache/origin/test_init.py @@ -149,21 +149,8 @@ def test_store_purge_keys_organization_project_add_survives_events_committed_sta ): # Regression guard for issue #19911. # - # `store_purge_keys` skips emitting a purge for a dirty object whose - # `committed_state` is a subset of `_NON_CACHE_RELEVANT_ATTRS` (the - # perf optimizations in PRs #19898 and #19910). Before this fix, - # `add_organization_project` relied on `flag_dirty(org)` to trigger an - # Organization-dirty purge — but the manage view then recorded an event - # on the organization, setting `event.source = org` and back-populating - # the `events` dynamic collection, which put `'events'` in the - # Organization's committed_state. The skip then masked the only purge - # for `org/{orgname}`, leaving the CDN entry stale. - # - # The fix makes the purge come from the new OrganizationProject row - # (session.new, not session.dirty) — the skip applies only to dirty - # objects, so a `session.new` row is unaffected. This test pins down - # that behavior: the `org/{name}` purge must still fire even when the - # Organization has `events` in its committed_state at flush time. + # We should purge `org/{name}` even when the Organization's + # `committed_state` includes `events` (or any other non-cache-relevant attr). organization = OrganizationFactory.create() project = ProjectFactory.create() db_request.db.flush()