diff --git a/tests/unit/cache/origin/test_init.py b/tests/unit/cache/origin/test_init.py index e4ecfdfa5140..881a6adf9e41 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,80 @@ 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_add_survives_events_committed_state( + app_config, db_request +): + # Regression guard for issue #19911. + # + # 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() + 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 +): + # 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: