diff --git a/dev/flake8/checkers.py b/dev/flake8/checkers.py index 82556bb5166e..14118bd0a888 100644 --- a/dev/flake8/checkers.py +++ b/dev/flake8/checkers.py @@ -23,6 +23,10 @@ from typing import Any WH001_msg = "WH001 Prefer `urllib3.util.parse_url` over `urllib.parse.urlparse`" +WH002_msg = ( + "WH002 Prefer `sqlalchemy.orm.relationship(back_populates=...)` " + "over `sqlalchemy.orm.relationship(backref=...)`" +) class WarehouseVisitor(ast.NodeVisitor): @@ -30,6 +34,29 @@ def __init__(self, filename: str) -> None: self.errors: list[tuple[int, int, str]] = [] self.filename = filename + def check_for_backref(self, node) -> None: + def _check_keywords(keywords: list[ast.keyword]) -> None: + for kw in keywords: + if kw.arg == "backref": + self.errors.append((kw.lineno, kw.col_offset, WH002_msg)) + + # Nodes can be either Attribute or Name, and depending on the type + # of node, the value.func can be either an attr or an id. + # TODO: This is aching for a better way to do this. + if isinstance(node.value, ast.Call): + if ( + isinstance(node.value.func, ast.Attribute) + and node.value.func.attr == "relationship" + and isinstance(node.value.keywords, list) + ): + _check_keywords(node.value.keywords) + elif ( + isinstance(node.value.func, ast.Name) + and node.value.func.id == "relationship" + and isinstance(node.value.keywords, list) + ): + _check_keywords(node.value.keywords) + def visit_Name(self, node: ast.Name) -> None: # noqa: N802 if node.id == "urlparse": self.errors.append((node.lineno, node.col_offset, WH001_msg)) @@ -46,6 +73,14 @@ def visit_Attribute(self, node: ast.Attribute) -> None: # noqa: N802 self.generic_visit(node) + def visit_Assign(self, node: ast.Assign) -> None: # noqa: N802 + self.check_for_backref(node) + self.generic_visit(node) + + def visit_AnnAssign(self, node: ast.AnnAssign) -> None: # noqa: N802 + self.check_for_backref(node) + self.generic_visit(node) + class WarehouseCheck: def __init__(self, tree: ast.AST, filename: str) -> None: diff --git a/tests/unit/packaging/test_models.py b/tests/unit/packaging/test_models.py index 470100fdf487..70e1070bbf67 100644 --- a/tests/unit/packaging/test_models.py +++ b/tests/unit/packaging/test_models.py @@ -570,6 +570,16 @@ def test_trusted_published_mixed(self, db_session): assert not release.trusted_published + def test_description_relationship(self, db_request): + """When a Release is deleted, the Description is also deleted.""" + release = DBReleaseFactory.create() # also creates a Description + description = release.description + + db_request.db.delete(release) + + assert release in db_request.db.deleted + assert description in db_request.db.deleted + class TestFile: def test_requires_python(self, db_session): diff --git a/tests/unit/packaging/test_tasks.py b/tests/unit/packaging/test_tasks.py index 5b3c0e24b908..0873f70577ce 100644 --- a/tests/unit/packaging/test_tasks.py +++ b/tests/unit/packaging/test_tasks.py @@ -389,13 +389,12 @@ def test_update_description_html(monkeypatch, db_request): def test_update_release_description(db_request): - release = ReleaseFactory.create() description = DescriptionFactory.create( - release=release, raw="rst\n===\n\nbody text", html="", rendered_by="0.0", ) + release = ReleaseFactory.create(description=description) task = pretend.stub() update_release_description(task, db_request, release.id) diff --git a/warehouse/accounts/models.py b/warehouse/accounts/models.py index 4aae9e6dcc01..49f263745d31 100644 --- a/warehouse/accounts/models.py +++ b/warehouse/accounts/models.py @@ -43,6 +43,12 @@ if TYPE_CHECKING: from warehouse.macaroons.models import Macaroon from warehouse.oidc.models import PendingOIDCPublisher + from warehouse.organizations.models import ( + Organization, + OrganizationApplication, + OrganizationRole, + Team, + ) from warehouse.packaging.models import Project @@ -97,33 +103,42 @@ class User(SitemapMixin, HasEvents, db.Model): last_totp_value: Mapped[str | None] webauthn: Mapped[list[WebAuthn]] = orm.relationship( - "WebAuthn", backref="user", cascade="all, delete-orphan", lazy=True + back_populates="user", cascade="all, delete-orphan", lazy=True ) recovery_codes: Mapped[list[RecoveryCode]] = orm.relationship( - "RecoveryCode", backref="user", cascade="all, delete-orphan", lazy="dynamic" + back_populates="user", cascade="all, delete-orphan", lazy="dynamic" ) emails: Mapped[list[Email]] = orm.relationship( - "Email", backref="user", cascade="all, delete-orphan", lazy=False + back_populates="user", cascade="all, delete-orphan", lazy=False ) macaroons: Mapped[list[Macaroon]] = orm.relationship( - "Macaroon", cascade="all, delete-orphan", lazy=True, order_by="Macaroon.created.desc()", ) + organization_applications: Mapped[list[OrganizationApplication]] = orm.relationship( + back_populates="submitted_by", + ) + + organizations: Mapped[list[Organization]] = orm.relationship( + secondary="organization_roles", + back_populates="users", + lazy=True, + order_by="Organization.name", + viewonly=True, + ) + pending_oidc_publishers: Mapped[list[PendingOIDCPublisher]] = orm.relationship( - "PendingOIDCPublisher", - backref="added_by", + back_populates="added_by", cascade="all, delete-orphan", lazy=True, ) projects: Mapped[list[Project]] = orm.relationship( - "Project", secondary="roles", back_populates="users", lazy=True, @@ -131,6 +146,21 @@ class User(SitemapMixin, HasEvents, db.Model): order_by="Project.normalized_name", ) + organization_roles: Mapped[list[OrganizationRole]] = orm.relationship( + back_populates="user", + cascade="all, delete-orphan", + lazy=True, + viewonly=True, + ) + + teams: Mapped[list[Team]] = orm.relationship( + secondary="team_roles", + back_populates="members", + lazy=True, + viewonly=True, + order_by="Team.name", + ) + @property def primary_email(self): primaries = [x for x in self.emails if x.primary] @@ -243,6 +273,7 @@ class WebAuthn(db.Model): nullable=False, index=True, ) + user: Mapped[User] = orm.relationship(back_populates="webauthn") label: Mapped[str] credential_id: Mapped[str] = mapped_column(unique=True) public_key: Mapped[str | None] = mapped_column(unique=True) @@ -258,6 +289,7 @@ class RecoveryCode(db.Model): nullable=False, index=True, ) + user: Mapped[User] = orm.relationship(back_populates="recovery_codes") code: Mapped[str] = mapped_column(String(length=128)) generated: Mapped[datetime_now] burned: Mapped[datetime.datetime | None] @@ -281,6 +313,7 @@ class Email(db.ModelBase): PG_UUID(as_uuid=True), ForeignKey("users.id", deferrable=True, initially="DEFERRED"), ) + user: Mapped[User] = orm.relationship(back_populates="emails") email: Mapped[str] = mapped_column(String(length=254)) primary: Mapped[bool] verified: Mapped[bool] diff --git a/warehouse/email/ses/models.py b/warehouse/email/ses/models.py index 9b454cdf7e46..c7e3a5753153 100644 --- a/warehouse/email/ses/models.py +++ b/warehouse/email/ses/models.py @@ -248,9 +248,8 @@ class EmailMessage(db.Model): missing: Mapped[bool_false] # Relationships! - events = orm.relationship( - "Event", - backref="email", + events: Mapped[list["Event"]] = orm.relationship( + back_populates="email", cascade="all, delete-orphan", lazy=False, order_by=lambda: Event.created, @@ -275,6 +274,10 @@ class Event(db.Model): ), index=True, ) + email: Mapped[EmailMessage] = orm.relationship( + back_populates="events", + lazy=False, + ) event_id: Mapped[str] = mapped_column(unique=True, index=True) event_type: Mapped[Enum] = mapped_column( diff --git a/warehouse/oidc/models/_core.py b/warehouse/oidc/models/_core.py index fc770989914b..8330449a4f4f 100644 --- a/warehouse/oidc/models/_core.py +++ b/warehouse/oidc/models/_core.py @@ -13,19 +13,22 @@ from __future__ import annotations from collections.abc import Callable -from typing import Any, TypeVar +from typing import TYPE_CHECKING, Any, TypeVar import sentry_sdk from sqlalchemy import ForeignKey, String, orm from sqlalchemy.dialects.postgresql import UUID -from sqlalchemy.orm import mapped_column +from sqlalchemy.orm import Mapped, mapped_column from warehouse import db -from warehouse.macaroons.models import Macaroon from warehouse.oidc.errors import InvalidPublisherError from warehouse.oidc.interfaces import SignedClaims -from warehouse.packaging.models import Project + +if TYPE_CHECKING: + from warehouse.accounts.models import User + from warehouse.macaroons.models import Macaroon + from warehouse.packaging.models import Project C = TypeVar("C") @@ -241,12 +244,13 @@ def publisher_url(self, claims=None) -> str | None: # pragma: no cover class OIDCPublisher(OIDCPublisherMixin, db.Model): __tablename__ = "oidc_publishers" - projects = orm.relationship( - Project, + projects: Mapped[list[Project]] = orm.relationship( secondary=OIDCPublisherProjectAssociation.__table__, - backref="oidc_publishers", + back_populates="oidc_publishers", + ) + macaroons: Mapped[list[Macaroon]] = orm.relationship( + cascade="all, delete-orphan", lazy=True ) - macaroons = orm.relationship(Macaroon, cascade="all, delete-orphan", lazy=True) __mapper_args__ = { "polymorphic_identity": "oidc_publishers", @@ -266,6 +270,7 @@ class PendingOIDCPublisher(OIDCPublisherMixin, db.Model): added_by_id = mapped_column( UUID(as_uuid=True), ForeignKey("users.id"), nullable=False, index=True ) + added_by: Mapped[User] = orm.relationship(back_populates="pending_oidc_publishers") __mapper_args__ = { "polymorphic_identity": "pending_oidc_publishers", diff --git a/warehouse/organizations/models.py b/warehouse/organizations/models.py index 0a0d57e2820f..bd27dc1e8526 100644 --- a/warehouse/organizations/models.py +++ b/warehouse/organizations/models.py @@ -82,8 +82,10 @@ class OrganizationRole(db.Model): ForeignKey("organizations.id", onupdate="CASCADE", ondelete="CASCADE"), ) - user: Mapped[User] = relationship(lazy=False) - organization: Mapped[Organization] = relationship(lazy=False) + user: Mapped[User] = relationship(back_populates="organization_roles", lazy=False) + organization: Mapped[Organization] = relationship( + back_populates="roles", lazy=False + ) class OrganizationProject(db.Model): @@ -270,10 +272,16 @@ class Organization(OrganizationMixin, HasEvents, db.Model): onupdate=func.now(), comment="Datetime the organization was approved by administrators.", ) + application: Mapped[OrganizationApplication] = relationship( + back_populates="organization" + ) users: Mapped[list[User]] = relationship( - secondary=OrganizationRole.__table__, backref="organizations", viewonly=True + secondary=OrganizationRole.__table__, + back_populates="organizations", + viewonly=True, ) + roles: Mapped[list[OrganizationRole]] = relationship(back_populates="organization") teams: Mapped[list[Team]] = relationship( back_populates="organization", order_by=lambda: Team.name.asc(), @@ -471,9 +479,11 @@ class OrganizationApplication(OrganizationMixin, db.Model): comment="If the request was approved, ID of resulting Organization", ) - submitted_by: Mapped[User] = relationship(backref="organization_applications") + submitted_by: Mapped[User] = relationship( + back_populates="organization_applications" + ) organization: Mapped[Organization] = relationship( - backref="application", viewonly=True + back_populates="application", viewonly=True ) @@ -644,10 +654,10 @@ class Team(HasEvents, db.Model): lazy=False, back_populates="teams" ) members: Mapped[list[User]] = relationship( - secondary=TeamRole.__table__, backref="teams", viewonly=True + secondary=TeamRole.__table__, back_populates="teams", viewonly=True ) projects: Mapped[list[Project]] = relationship( - secondary=TeamProjectRole.__table__, backref="teams", viewonly=True + secondary=TeamProjectRole.__table__, back_populates="team", viewonly=True ) def record_event(self, *, tag, request: Request = None, additional=None): diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 8c97f4a08717..8fe2a0ea6083 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -12,6 +12,7 @@ from __future__ import annotations import enum +import typing from collections import OrderedDict from uuid import UUID @@ -58,6 +59,7 @@ OrganizationProject, OrganizationRole, OrganizationRoleType, + Team, TeamProjectRole, ) from warehouse.sitemap.models import SitemapMixin @@ -65,6 +67,9 @@ from warehouse.utils.attrs import make_repr from warehouse.utils.db.types import bool_false, datetime_now +if typing.TYPE_CHECKING: + from warehouse.oidc.models import OIDCPublisher + class Role(db.Model): __tablename__ = "roles" @@ -175,6 +180,12 @@ class Project(SitemapMixin, TwoFactorRequireable, HasEvents, db.Model): BigInteger, server_default=sql.text("0") ) + oidc_publishers: Mapped[list[OIDCPublisher]] = orm.relationship( + secondary="oidc_publisher_project_association", + back_populates="projects", + passive_deletes=True, + ) + organization: Mapped[Organization] = orm.relationship( secondary=OrganizationProject.__table__, back_populates="projects", @@ -185,6 +196,11 @@ class Project(SitemapMixin, TwoFactorRequireable, HasEvents, db.Model): back_populates="project", passive_deletes=True, ) + team: Mapped[Team] = orm.relationship( + secondary=TeamProjectRole.__table__, + back_populates="projects", + viewonly=True, + ) team_project_roles: Mapped[list[TeamProjectRole]] = orm.relationship( back_populates="project", passive_deletes=True, @@ -193,7 +209,6 @@ class Project(SitemapMixin, TwoFactorRequireable, HasEvents, db.Model): secondary=Role.__table__, back_populates="projects", viewonly=True ) releases: Mapped[list[Release]] = orm.relationship( - backref="project", cascade="all, delete-orphan", order_by=lambda: Release._pypi_ordering.desc(), passive_deletes=True, @@ -365,6 +380,7 @@ class Dependency(db.Model): release_id: Mapped[UUID] = mapped_column( ForeignKey("releases.id", onupdate="CASCADE", ondelete="CASCADE"), ) + release: Mapped[Release] = orm.relationship(back_populates="dependencies") kind: Mapped[int | None] specifier: Mapped[str | None] @@ -387,6 +403,8 @@ class Description(db.Model): html: Mapped[str] rendered_by: Mapped[str] + release: Mapped[Release] = orm.relationship(back_populates="description") + class ReleaseURL(db.Model): __tablename__ = "release_urls" @@ -403,6 +421,7 @@ class ReleaseURL(db.Model): ForeignKey("releases.id", onupdate="CASCADE", ondelete="CASCADE"), index=True, ) + release: Mapped[Release] = orm.relationship(back_populates="_project_urls") name: Mapped[str] = mapped_column(String(32)) url: Mapped[str] @@ -428,6 +447,7 @@ def __table_args__(cls): # noqa project_id: Mapped[UUID] = mapped_column( ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), ) + project: Mapped[Project] = orm.relationship(back_populates="releases") version: Mapped[str] = mapped_column(Text) canonical_version: Mapped[str] = mapped_column() is_prerelease: Mapped[bool_false] @@ -450,14 +470,9 @@ def __table_args__(cls): # noqa index=True, ) description: Mapped[Description] = orm.relationship( - backref=orm.backref( - "release", - cascade="all, delete-orphan", - passive_deletes=True, - passive_updates=True, - single_parent=True, - uselist=False, - ), + back_populates="release", + cascade="all, delete-orphan", + single_parent=True, ) yanked: Mapped[bool_false] @@ -465,7 +480,6 @@ def __table_args__(cls): # noqa yanked_reason: Mapped[str] = mapped_column(server_default="") _classifiers: Mapped[list[Classifier]] = orm.relationship( - backref="project_releases", secondary="release_classifiers", order_by=Classifier.ordering, passive_deletes=True, @@ -473,7 +487,6 @@ def __table_args__(cls): # noqa classifiers = association_proxy("_classifiers", "classifier") _project_urls: Mapped[list[ReleaseURL]] = orm.relationship( - backref="release", collection_class=attribute_keyed_dict("name"), cascade="all, delete-orphan", order_by=lambda: ReleaseURL.name.asc(), @@ -486,8 +499,6 @@ def __table_args__(cls): # noqa ) files: Mapped[list[File]] = orm.relationship( - "File", - backref="release", cascade="all, delete-orphan", lazy="dynamic", order_by=lambda: File.filename, @@ -495,7 +506,6 @@ def __table_args__(cls): # noqa ) dependencies: Mapped[list[Dependency]] = orm.relationship( - backref="release", cascade="all, delete-orphan", passive_deletes=True, ) @@ -652,6 +662,7 @@ def __table_args__(cls): # noqa release_id: Mapped[UUID] = mapped_column( ForeignKey("releases.id", onupdate="CASCADE", ondelete="CASCADE"), ) + release: Mapped[Release] = orm.relationship(back_populates="files") python_version: Mapped[str] requires_python: Mapped[str | None] packagetype: Mapped[PackageType] = mapped_column()