From 3962dc0c7cd7c48b60cc7daa55f236181d493561 Mon Sep 17 00:00:00 2001 From: Ernest W Durbin III Date: Sat, 27 Oct 2018 13:23:57 -0400 Subject: [PATCH 01/13] Migrate to UUID Primary Key for Project and Release models --- ...6a223_migrate_projects_and_releases_to_.py | 169 ++++++++++++++++++ warehouse/packaging/models.py | 45 ++--- 2 files changed, 188 insertions(+), 26 deletions(-) create mode 100644 warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py diff --git a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py new file mode 100644 index 000000000000..2bd260110be2 --- /dev/null +++ b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py @@ -0,0 +1,169 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +migrate projects and releases to surrogate primary_key + +Revision ID: ee5b8f66a223 +Revises: e82c3a017d60 +Create Date: 2018-10-27 16:31:38.859484 +""" + +from alembic import op +import sqlalchemy as sa +from sqlalchemy.dialects import postgresql + +revision = "ee5b8f66a223" +down_revision = "e82c3a017d60" + + +def upgrade(): + op.add_column( + "packages", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + ) + op.add_column( + "releases", + sa.Column( + "id", + postgresql.UUID(as_uuid=True), + server_default=sa.text("gen_random_uuid()"), + nullable=False, + ), + ) + op.add_column( + "releases", + sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=True), + ) + op.add_column( + "release_files", + sa.Column("release_id", postgresql.UUID(as_uuid=True), nullable=True), + ) + op.add_column( + "release_dependencies", + sa.Column("release_id", postgresql.UUID(as_uuid=True), nullable=True), + ) + op.add_column( + "release_classifiers", + sa.Column("release_id", postgresql.UUID(as_uuid=True), nullable=True), + ) + + op.execute( + """ UPDATE releases + SET project_id = packages.id + FROM packages + WHERE releases.name = packages.name + """ + ) + op.execute( + """ UPDATE release_files + SET release_id = releases.id + FROM releases + WHERE + release_files.name = releases.name + AND release_files.version = releases.version + """ + ) + op.execute( + """ DELETE FROM release_dependencies + WHERE + name IS NULL AND version IS NULL + """ + ) + op.execute( + """ UPDATE release_dependencies + SET release_id = releases.id + FROM releases + WHERE + release_dependencies.name = releases.name + AND release_dependencies.version = releases.version + """ + ) + op.execute( + """ UPDATE release_classifiers + SET release_id = releases.id + FROM releases + WHERE + release_classifiers.name = releases.name + AND release_classifiers.version = releases.version + """ + ) + + op.alter_column("releases", "project_id", nullable=False) + op.alter_column("release_files", "release_id", nullable=False) + op.alter_column("release_dependencies", "release_id", nullable=False) + op.alter_column("release_classifiers", "release_id", nullable=False) + + op.drop_constraint( + "release_classifiers_name_fkey", "release_classifiers", type_="foreignkey" + ) + op.drop_constraint( + "release_dependencies_name_fkey", "release_dependencies", type_="foreignkey" + ) + op.drop_constraint("release_files_name_fkey", "release_files", type_="foreignkey") + op.drop_constraint("releases_name_fkey", "releases", type_="foreignkey") + + op.execute("ALTER TABLE packages DROP CONSTRAINT packages_pkey CASCADE") + op.create_primary_key(None, "packages", ["id"]) + op.create_index( + "release_normalized_name_version_idx", + "releases", + [sa.text("normalize_pep426_name(name)"), "version"], + unique=True, + ) + op.execute("ALTER TABLE releases DROP CONSTRAINT releases_pkey CASCADE") + op.create_primary_key(None, "releases", ["id"]) + + op.create_foreign_key( + None, + "releases", + "packages", + ["project_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + op.create_foreign_key( + None, + "release_files", + "releases", + ["release_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + op.create_foreign_key( + None, + "release_dependencies", + "releases", + ["release_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + op.create_foreign_key( + None, + "release_classifiers", + "releases", + ["release_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + + +def downgrade(): + raise RuntimeError("Order No. 227 - Ни шагу назад!") diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 01a7c4dfc6da..b83ee51fa76d 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -65,9 +65,11 @@ class Role(db.Model): CIText, ForeignKey("accounts_user.username", onupdate="CASCADE", ondelete="CASCADE"), ) - package_name = Column( - Text, ForeignKey("packages.name", onupdate="CASCADE", ondelete="CASCADE") + project_id = Column( + ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, ) + package_name = Column(Text) user = orm.relationship(User, lazy=False) project = orm.relationship("Project", lazy=False) @@ -110,7 +112,8 @@ class Project(SitemapMixin, db.ModelBase): __repr__ = make_repr("name") - name = Column(Text, primary_key=True, nullable=False) + id = Column(UUID(as_uuid=True), primary_key=True, nullable=False) + name = Column(Text, nullable=False) normalized_name = orm.column_property(func.normalize_pep426_name(name)) created = Column( DateTime(timezone=False), nullable=False, server_default=sql.func.now() @@ -233,14 +236,12 @@ class Dependency(db.Model): __table_args__ = ( Index("rel_dep_name_version_kind_idx", "name", "version", "kind"), ForeignKeyConstraint( - ["name", "version"], - ["releases.name", "releases.version"], - onupdate="CASCADE", - ondelete="CASCADE", + ["release_id"], ["releases.id"], onupdate="CASCADE", ondelete="CASCADE" ), ) __repr__ = make_repr("name", "version", "kind", "specifier") + release_id = Column(ForeignKey("releases.id"), nullable=False) name = Column(Text) version = Column(Text) kind = Column(Integer) @@ -251,9 +252,7 @@ def _dependency_relation(kind): return orm.relationship( "Dependency", primaryjoin=lambda: sql.and_( - Release.name == Dependency.name, - Release.version == Dependency.version, - Dependency.kind == kind.value, + Release.id == Dependency.release_id, Dependency.kind == kind.value ), viewonly=True, ) @@ -266,6 +265,9 @@ class Release(db.ModelBase): @declared_attr def __table_args__(cls): # noqa return ( + ForeignKeyConstraint( + ["project_id"], ["packages.id"], onupdate="CASCADE", ondelete="CASCADE" + ), Index("release_created_idx", cls.created.desc()), Index("release_name_created_idx", cls.name, cls.created.desc()), Index("release_version_idx", cls.version), @@ -275,12 +277,10 @@ def __table_args__(cls): # noqa __parent__ = dotted_navigator("project") __name__ = dotted_navigator("version") - name = Column( - Text, - ForeignKey("packages.name", onupdate="CASCADE", ondelete="CASCADE"), - primary_key=True, - ) - version = Column(Text, primary_key=True) + id = Column(UUID(as_uuid=True), primary_key=True, nullable=False) + project_id = Column(ForeignKey("packages.id"), nullable=False) + name = Column(Text, nullable=False) + version = Column(Text, nullable=False) canonical_version = Column(Text, nullable=False) is_prerelease = orm.column_property(func.pep440_is_prerelease(version)) author = Column(Text) @@ -425,10 +425,7 @@ class File(db.Model): def __table_args__(cls): # noqa return ( ForeignKeyConstraint( - ["name", "version"], - ["releases.name", "releases.version"], - onupdate="CASCADE", - ondelete="CASCADE", + ["release_id"], ["releases.id"], onupdate="CASCADE", ondelete="CASCADE" ), CheckConstraint("sha256_digest ~* '^[A-F0-9]{64}$'"), CheckConstraint("blake2_256_digest ~* '^[A-F0-9]{64}$'"), @@ -447,6 +444,7 @@ def __table_args__(cls): # noqa ), ) + release_id = Column(ForeignKey("releases.id"), nullable=False) name = Column(Text) version = Column(Text) python_version = Column(Text) @@ -503,15 +501,10 @@ class Filename(db.ModelBase): release_classifiers = Table( "release_classifiers", db.metadata, + Column("release_id", ForeignKey("releases.id"), nullable=False), Column("name", Text()), Column("version", Text()), Column("trove_id", Integer(), ForeignKey("trove_classifiers.id")), - ForeignKeyConstraint( - ["name", "version"], - ["releases.name", "releases.version"], - onupdate="CASCADE", - ondelete="CASCADE", - ), Index("rel_class_name_version_idx", "name", "version"), Index("rel_class_trove_id_idx", "trove_id"), Index("rel_class_version_id_idx", "version"), From 074871e41d664570090800ed0598a9934677b9e5 Mon Sep 17 00:00:00 2001 From: Ernest W Durbin III Date: Sat, 27 Oct 2018 18:02:43 -0400 Subject: [PATCH 02/13] fixup factories for Project and Release, sync Role model to migration --- tests/common/db/packaging.py | 3 +++ ...6a223_migrate_projects_and_releases_to_.py | 22 +++++++++++++++++++ 2 files changed, 25 insertions(+) diff --git a/tests/common/db/packaging.py b/tests/common/db/packaging.py index cd72f6d557e0..bd1c86658f88 100644 --- a/tests/common/db/packaging.py +++ b/tests/common/db/packaging.py @@ -12,6 +12,7 @@ import datetime import hashlib +import uuid import factory import factory.fuzzy @@ -36,6 +37,7 @@ class ProjectFactory(WarehouseFactory): class Meta: model = Project + id = factory.LazyFunction(uuid.uuid4) name = factory.fuzzy.FuzzyText(length=12) @@ -43,6 +45,7 @@ class ReleaseFactory(WarehouseFactory): class Meta: model = Release + id = factory.LazyFunction(uuid.uuid4) name = factory.LazyAttribute(lambda o: o.project.name) project = factory.SubFactory(ProjectFactory) version = factory.Sequence(lambda n: str(n) + ".0") diff --git a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py index 2bd260110be2..f1f3bd89072f 100644 --- a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py +++ b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py @@ -44,6 +44,10 @@ def upgrade(): nullable=False, ), ) + op.add_column( + "roles", + sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=True), + ) op.add_column( "releases", sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=True), @@ -68,6 +72,14 @@ def upgrade(): WHERE releases.name = packages.name """ ) + op.execute( + """ UPDATE roles + SET project_id = packages.id + FROM packages + WHERE + packages.name = roles.package_name + """ + ) op.execute( """ UPDATE release_files SET release_id = releases.id @@ -103,6 +115,7 @@ def upgrade(): ) op.alter_column("releases", "project_id", nullable=False) + op.alter_column("roles", "package_name", nullable=False) op.alter_column("release_files", "release_id", nullable=False) op.alter_column("release_dependencies", "release_id", nullable=False) op.alter_column("release_classifiers", "release_id", nullable=False) @@ -136,6 +149,15 @@ def upgrade(): onupdate="CASCADE", ondelete="CASCADE", ) + op.create_foreign_key( + None, + "roles", + "packages", + ["project_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) op.create_foreign_key( None, "release_files", From 9bd1a4c2d73ac276eb2e1cecaacdacbd8f04e0b9 Mon Sep 17 00:00:00 2001 From: Ernest W Durbin III Date: Sat, 27 Oct 2018 18:08:06 -0400 Subject: [PATCH 03/13] make reformat --- .../versions/ee5b8f66a223_migrate_projects_and_releases_to_.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py index f1f3bd89072f..e5d289c99273 100644 --- a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py +++ b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py @@ -45,8 +45,7 @@ def upgrade(): ), ) op.add_column( - "roles", - sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=True), + "roles", sa.Column("project_id", postgresql.UUID(as_uuid=True), nullable=True) ) op.add_column( "releases", From 961b3b73b9b466dbeed6c157152f7b0466753047 Mon Sep 17 00:00:00 2001 From: Ernest W Durbin III Date: Sun, 28 Oct 2018 10:48:42 -0400 Subject: [PATCH 04/13] fix at least one test --- tests/common/db/packaging.py | 1 + .../ee5b8f66a223_migrate_projects_and_releases_to_.py | 3 ++- warehouse/packaging/models.py | 10 +++++----- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/common/db/packaging.py b/tests/common/db/packaging.py index bd1c86658f88..186bc49662d9 100644 --- a/tests/common/db/packaging.py +++ b/tests/common/db/packaging.py @@ -101,6 +101,7 @@ class Meta: name = factory.fuzzy.FuzzyText(length=12) version = factory.Sequence(lambda n: str(n) + ".0") + release = project = factory.SubFactory(ReleaseFactory) kind = factory.fuzzy.FuzzyChoice(int(kind) for kind in DependencyKind) specifier = factory.fuzzy.FuzzyText(length=12) diff --git a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py index e5d289c99273..2dccf506c47e 100644 --- a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py +++ b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py @@ -114,11 +114,12 @@ def upgrade(): ) op.alter_column("releases", "project_id", nullable=False) - op.alter_column("roles", "package_name", nullable=False) op.alter_column("release_files", "release_id", nullable=False) op.alter_column("release_dependencies", "release_id", nullable=False) op.alter_column("release_classifiers", "release_id", nullable=False) + op.drop_column("roles", "package_name") + op.drop_constraint( "release_classifiers_name_fkey", "release_classifiers", type_="foreignkey" ) diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index b83ee51fa76d..2fca8475b492 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -53,10 +53,7 @@ class Role(db.Model): __tablename__ = "roles" - __table_args__ = ( - Index("roles_pack_name_idx", "package_name"), - Index("roles_user_name_idx", "user_name"), - ) + __table_args__ = (Index("roles_user_name_idx", "user_name"),) __repr__ = make_repr("role_name", "user_name", "package_name") @@ -69,11 +66,14 @@ class Role(db.Model): ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, ) - package_name = Column(Text) user = orm.relationship(User, lazy=False) project = orm.relationship("Project", lazy=False) + @property + def package_name(self): + return project.name + def __gt__(self, other): """ Temporary hack to allow us to only display the 'highest' role when From df3e04bc6eb39650ab24998145874b70fd608c55 Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Sun, 28 Oct 2018 15:30:07 -0400 Subject: [PATCH 05/13] Fix fallout of Role.package_name removal --- warehouse/admin/views/users.py | 2 +- warehouse/manage/views.py | 8 ++++---- warehouse/packaging/models.py | 4 ---- 3 files changed, 5 insertions(+), 9 deletions(-) diff --git a/warehouse/admin/views/users.py b/warehouse/admin/views/users.py index 394a8fb409d3..b1c796777c16 100644 --- a/warehouse/admin/views/users.py +++ b/warehouse/admin/views/users.py @@ -108,7 +108,7 @@ def user_detail(request): request.db.query(Role) .join(User) .filter(Role.user == user) - .order_by(Role.role_name, Role.package_name) + .order_by(Role.role_name, Role.project_id) .all() ) diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 61a0f62c26f9..8d2e7c918770 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -42,18 +42,18 @@ def user_projects(request): """ Return all the projects for which the user is a sole owner """ projects_owned = ( - request.db.query(Project) + request.db.query(Project.id) .join(Role.project) .filter(Role.role_name == "Owner", Role.user == request.user) .subquery() ) with_sole_owner = ( - request.db.query(Role.package_name) + request.db.query(Role.package_id) .join(projects_owned) .filter(Role.role_name == "Owner") - .group_by(Role.package_name) - .having(func.count(Role.package_name) == 1) + .group_by(Role.package_id) + .having(func.count(Role.package_id) == 1) .subquery() ) diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 2fca8475b492..fcced6eacd12 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -70,10 +70,6 @@ class Role(db.Model): user = orm.relationship(User, lazy=False) project = orm.relationship("Project", lazy=False) - @property - def package_name(self): - return project.name - def __gt__(self, other): """ Temporary hack to allow us to only display the 'highest' role when From 7fc001b3c123c37b2d788660e48eaf98fe770755 Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Mon, 5 Nov 2018 13:53:35 -0500 Subject: [PATCH 06/13] Remove Denormalized name and version fields from all models (#4974) --- tests/common/db/packaging.py | 6 +- tests/unit/admin/views/test_blacklist.py | 4 +- tests/unit/legacy/api/test_json.py | 7 +- tests/unit/legacy/api/xmlrpc/test_xmlrpc.py | 8 +- tests/unit/manage/test_views.py | 144 +++++++++--------- tests/unit/packaging/test_models.py | 3 +- tests/unit/utils/test_project.py | 17 ++- warehouse/admin/views/blacklist.py | 16 +- warehouse/admin/views/projects.py | 2 +- warehouse/legacy/api/json.py | 6 +- warehouse/legacy/api/simple.py | 12 +- warehouse/legacy/api/xmlrpc/views.py | 13 +- warehouse/manage/views.py | 15 +- ...6a223_migrate_projects_and_releases_to_.py | 75 ++++++++- warehouse/packaging/models.py | 36 ++--- warehouse/packaging/tasks.py | 6 +- warehouse/search/tasks.py | 27 ++-- warehouse/views.py | 18 ++- 18 files changed, 242 insertions(+), 173 deletions(-) diff --git a/tests/common/db/packaging.py b/tests/common/db/packaging.py index 186bc49662d9..dab5fd972dcd 100644 --- a/tests/common/db/packaging.py +++ b/tests/common/db/packaging.py @@ -46,7 +46,6 @@ class Meta: model = Release id = factory.LazyFunction(uuid.uuid4) - name = factory.LazyAttribute(lambda o: o.project.name) project = factory.SubFactory(ProjectFactory) version = factory.Sequence(lambda n: str(n) + ".0") canonical_version = factory.LazyAttribute( @@ -61,7 +60,6 @@ class FileFactory(WarehouseFactory): class Meta: model = File - name = factory.LazyAttribute(lambda o: o.release.name) release = factory.SubFactory(ReleaseFactory) python_version = "source" md5_digest = factory.LazyAttribute( @@ -99,9 +97,7 @@ class DependencyFactory(WarehouseFactory): class Meta: model = Dependency - name = factory.fuzzy.FuzzyText(length=12) - version = factory.Sequence(lambda n: str(n) + ".0") - release = project = factory.SubFactory(ReleaseFactory) + release = factory.SubFactory(ReleaseFactory) kind = factory.fuzzy.FuzzyChoice(int(kind) for kind in DependencyKind) specifier = factory.fuzzy.FuzzyText(length=12) diff --git a/tests/unit/admin/views/test_blacklist.py b/tests/unit/admin/views/test_blacklist.py index b96d70643d93..60d066c74f0f 100644 --- a/tests/unit/admin/views/test_blacklist.py +++ b/tests/unit/admin/views/test_blacklist.py @@ -211,9 +211,7 @@ def test_adds_blacklist_with_deletes(self, db_request): project = ProjectFactory.create(name="foo") release = ReleaseFactory.create(project=project) - FileFactory.create( - name=project.name, version=release.version, filename="who cares" - ) + FileFactory.create(release=release, filename="who cares") RoleFactory.create(project=project, user=db_request.user) views.add_blacklist(db_request) diff --git a/tests/unit/legacy/api/test_json.py b/tests/unit/legacy/api/test_json.py index 0acd678c2c0f..1dea89554f3b 100644 --- a/tests/unit/legacy/api/test_json.py +++ b/tests/unit/legacy/api/test_json.py @@ -194,8 +194,7 @@ def test_detail_renders(self, pyramid_config, db_request, db_session): for urlspec in project_urls: db_session.add( Dependency( - name=releases[3].project.name, - version="3.0", + release=releases[3], kind=DependencyKind.project_url.value, specifier=urlspec, ) @@ -460,7 +459,9 @@ def test_normalizing_redirects(self, db_request): assert isinstance(resp, HTTPMovedPermanently) assert db_request.route_path.calls == [ pretend.call( - "legacy.api.json.release", name=release.name, version=release.version + "legacy.api.json.release", + name=release.project.name, + version=release.version, ) ] assert resp.headers["Location"] == "/project/the-redirect" diff --git a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py index 705890aa073d..21c3141c40be 100644 --- a/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py +++ b/tests/unit/legacy/api/xmlrpc/test_xmlrpc.py @@ -785,7 +785,7 @@ def test_browse(db_request): expected_release._classifiers = classifiers assert set(xmlrpc.browse(db_request, ["Environment :: Other Environment"])) == { - (r.name, r.version) for r in releases + (r.project.name, r.version) for r in releases } assert set( xmlrpc.browse( @@ -795,7 +795,7 @@ def test_browse(db_request): "Development Status :: 5 - Production/Stable", ], ) - ) == {(expected_release.name, expected_release.version)} + ) == {(expected_release.project.name, expected_release.version)} assert set( xmlrpc.browse( db_request, @@ -805,7 +805,7 @@ def test_browse(db_request): "Programming Language :: Python", ], ) - ) == {(expected_release.name, expected_release.version)} + ) == {(expected_release.project.name, expected_release.version)} assert set( xmlrpc.browse( db_request, @@ -814,7 +814,7 @@ def test_browse(db_request): "Programming Language :: Python", ], ) - ) == {(expected_release.name, expected_release.version)} + ) == {(expected_release.project.name, expected_release.version)} def test_multicall(pyramid_request): diff --git a/tests/unit/manage/test_views.py b/tests/unit/manage/test_views.py index a3575c8ef5df..385518815a3b 100644 --- a/tests/unit/manage/test_views.py +++ b/tests/unit/manage/test_views.py @@ -22,7 +22,7 @@ from warehouse.manage import views from warehouse.accounts.interfaces import IUserService, IPasswordBreachedService -from warehouse.packaging.models import JournalEntry, Project, Role, User +from warehouse.packaging.models import JournalEntry, Project, File, Role, User from warehouse.utils.project import remove_documentation from ...common.db.accounts import EmailFactory @@ -30,6 +30,7 @@ JournalEntryFactory, ProjectFactory, ReleaseFactory, + FileFactory, RoleFactory, UserFactory, ) @@ -977,53 +978,51 @@ def test_delete_project_release_bad_confirm(self): ) ] - def test_delete_project_release_file(self, monkeypatch): - release_file = pretend.stub(filename="foo-bar.tar.gz", id=str(uuid.uuid4())) - release = pretend.stub(version="1.2.3", project=pretend.stub(name="foobar")) - request = pretend.stub( - POST={ - "confirm_project_name": release.project.name, - "file_id": release_file.id, - }, - method="POST", - db=pretend.stub( - delete=pretend.call_recorder(lambda a: None), - add=pretend.call_recorder(lambda a: None), - query=lambda a: pretend.stub( - filter=lambda *a: pretend.stub(one=lambda: release_file) - ), - ), - route_path=pretend.call_recorder(lambda *a, **kw: "/the-redirect"), - session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), - user=pretend.stub(), - remote_addr=pretend.stub(), + def test_delete_project_release_file(self, db_request): + user = UserFactory.create() + + project = ProjectFactory.create(name="foobar") + release = ReleaseFactory.create(project=project) + release_file = FileFactory.create( + release=release, filename=f"foobar-{release.version}.tar.gz" ) - journal_obj = pretend.stub() - journal_cls = pretend.call_recorder(lambda **kw: journal_obj) - monkeypatch.setattr(views, "JournalEntry", journal_cls) - view = views.ManageProjectRelease(release, request) + db_request.POST = { + "confirm_project_name": release.project.name, + "file_id": release_file.id, + } + db_request.method = ("POST",) + db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect") + db_request.session = pretend.stub( + flash=pretend.call_recorder(lambda *a, **kw: None) + ) + db_request.user = user + db_request.remote_addr = "1.2.3.4" + + view = views.ManageProjectRelease(release, db_request) result = view.delete_project_release_file() assert isinstance(result, HTTPSeeOther) assert result.headers["Location"] == "/the-redirect" - assert request.session.flash.calls == [ + assert db_request.session.flash.calls == [ pretend.call(f"Deleted file {release_file.filename!r}", queue="success") ] - assert request.db.delete.calls == [pretend.call(release_file)] - assert request.db.add.calls == [pretend.call(journal_obj)] - assert journal_cls.calls == [ - pretend.call( - name=release.project.name, - action=f"remove file {release_file.filename}", + + assert db_request.db.query(File).filter_by(id=release_file.id).first() is None + assert ( + db_request.db.query(JournalEntry) + .filter_by( + name=project.name, version=release.version, - submitted_by=request.user, - submitted_from=request.remote_addr, + action=f"remove file {release_file.filename}", + submitted_by=user, + submitted_from="1.2.3.4", ) - ] - assert request.route_path.calls == [ + .one() + ) + assert db_request.route_path.calls == [ pretend.call( "manage.project.release", project_name=release.project.name, @@ -1059,36 +1058,38 @@ def test_delete_project_release_file_no_confirm(self): ) ] - def test_delete_project_release_file_not_found(self): - release = pretend.stub(version="1.2.3", project=pretend.stub(name="foobar")) + def test_delete_project_release_file_not_found(self, db_request): + project = ProjectFactory.create(name="foobar") + release = ReleaseFactory.create(project=project) def no_result_found(): raise NoResultFound - request = pretend.stub( - POST={"confirm_project_name": "whatever"}, - method="POST", - db=pretend.stub( - delete=pretend.call_recorder(lambda a: None), - query=lambda a: pretend.stub( - filter=lambda *a: pretend.stub(one=no_result_found) - ), + db_request.POST = {"confirm_project_name": "whatever"} + db_request.method = "POST" + db_request.db = pretend.stub( + delete=pretend.call_recorder(lambda a: None), + query=lambda a: pretend.stub( + filter=lambda *a: pretend.stub(one=no_result_found) ), - route_path=pretend.call_recorder(lambda *a, **kw: "/the-redirect"), - session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), ) - view = views.ManageProjectRelease(release, request) + db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect") + db_request.session = pretend.stub( + flash=pretend.call_recorder(lambda *a, **kw: None) + ) + + view = views.ManageProjectRelease(release, db_request) result = view.delete_project_release_file() assert isinstance(result, HTTPSeeOther) assert result.headers["Location"] == "/the-redirect" - assert request.db.delete.calls == [] - assert request.session.flash.calls == [ + assert db_request.db.delete.calls == [] + assert db_request.session.flash.calls == [ pretend.call("Could not find file", queue="error") ] - assert request.route_path.calls == [ + assert db_request.route_path.calls == [ pretend.call( "manage.project.release", project_name=release.project.name, @@ -1096,37 +1097,38 @@ def no_result_found(): ) ] - def test_delete_project_release_file_bad_confirm(self): - release_file = pretend.stub(filename="foo-bar.tar.gz", id=str(uuid.uuid4())) - release = pretend.stub(version="1.2.3", project=pretend.stub(name="foobar")) - request = pretend.stub( - POST={"confirm_project_name": "invalid"}, - method="POST", - db=pretend.stub( - delete=pretend.call_recorder(lambda a: None), - query=lambda a: pretend.stub( - filter=lambda *a: pretend.stub(one=lambda: release_file) - ), - ), - route_path=pretend.call_recorder(lambda *a, **kw: "/the-redirect"), - session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)), + def test_delete_project_release_file_bad_confirm(self, db_request): + project = ProjectFactory.create(name="foobar") + release = ReleaseFactory.create(project=project, version="1.2.3") + release_file = FileFactory.create( + release=release, filename="foobar-1.2.3.tar.gz" ) - view = views.ManageProjectRelease(release, request) + + db_request.POST = { + "confirm_project_name": "invalid", + "file_id": str(release_file.id), + } + db_request.method = "POST" + db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect") + db_request.session = pretend.stub( + flash=pretend.call_recorder(lambda *a, **kw: None) + ) + + view = views.ManageProjectRelease(release, db_request) result = view.delete_project_release_file() assert isinstance(result, HTTPSeeOther) assert result.headers["Location"] == "/the-redirect" - - assert request.db.delete.calls == [] - assert request.session.flash.calls == [ + assert db_request.db.query(File).filter_by(id=release_file.id).one() + assert db_request.session.flash.calls == [ pretend.call( "Could not delete file - " + f"'invalid' is not the same as {release.project.name!r}", queue="error", ) ] - assert request.route_path.calls == [ + assert db_request.route_path.calls == [ pretend.call( "manage.project.release", project_name=release.project.name, diff --git a/tests/unit/packaging/test_models.py b/tests/unit/packaging/test_models.py index c3a275fac6a5..601169f57056 100644 --- a/tests/unit/packaging/test_models.py +++ b/tests/unit/packaging/test_models.py @@ -260,8 +260,7 @@ def test_urls(self, db_session, home_page, download_url, project_urls, expected) for urlspec in project_urls: db_session.add( Dependency( - name=release.project.name, - version=release.version, + release=release, kind=DependencyKind.project_url.value, specifier=urlspec, ) diff --git a/tests/unit/utils/test_project.py b/tests/unit/utils/test_project.py index 354dc4dd6737..b81872964597 100644 --- a/tests/unit/utils/test_project.py +++ b/tests/unit/utils/test_project.py @@ -95,9 +95,9 @@ def test_remove_project(db_request, flash): user = UserFactory.create() project = ProjectFactory.create(name="foo") release = ReleaseFactory.create(project=project) - FileFactory.create(name=project.name, version=release.version, filename="who cares") + FileFactory.create(release=release, filename="who cares") RoleFactory.create(user=user, project=project) - DependencyFactory.create(name=project.name, version=release.version) + DependencyFactory.create(release=release) db_request.user = user db_request.remote_addr = "192.168.1.1" @@ -113,13 +113,20 @@ def test_remove_project(db_request, flash): assert db_request.session.flash.calls == [] assert not (db_request.db.query(Role).filter(Role.project == project).count()) - assert not (db_request.db.query(File).filter(File.name == project.name).count()) assert not ( - db_request.db.query(Dependency).filter(Dependency.name == project.name).count() + db_request.db.query(File) + .join(Release) + .join(Project) + .filter(Release.project == project) + .count() ) assert not ( - db_request.db.query(Release).filter(Release.name == project.name).count() + db_request.db.query(Dependency) + .join(Release) + .filter(Release.project == project) + .count() ) + assert not (db_request.db.query(Release).filter(Release.project == project).count()) assert not ( db_request.db.query(Project).filter(Project.name == project.name).count() ) diff --git a/warehouse/admin/views/blacklist.py b/warehouse/admin/views/blacklist.py index 92ef84d79e93..e7377589ad81 100644 --- a/warehouse/admin/views/blacklist.py +++ b/warehouse/admin/views/blacklist.py @@ -88,11 +88,23 @@ def confirm_blacklist(request): .first() ) if project is not None: - releases = request.db.query(Release).filter(Release.name == project.name).all() - files = request.db.query(File).filter(File.name == project.name).all() + releases = ( + request.db.query(Release) + .join(Project) + .filter(Release.project == project) + .all() + ) + files = ( + request.db.query(File) + .join(Release) + .join(Project) + .filter(Release.project == project) + .all() + ) roles = ( request.db.query(Role) .join(User) + .join(Project) .filter(Role.project == project) .distinct(User.username) .order_by(User.username) diff --git a/warehouse/admin/views/projects.py b/warehouse/admin/views/projects.py index 1b566d388c4b..203287894312 100644 --- a/warehouse/admin/views/projects.py +++ b/warehouse/admin/views/projects.py @@ -174,7 +174,7 @@ def releases_list(project, request): def release_detail(release, request): journals = ( request.db.query(JournalEntry) - .filter(JournalEntry.name == release.name) + .filter(JournalEntry.name == release.project.name) .filter(JournalEntry.version == release.version) .order_by(JournalEntry.submitted_date.desc(), JournalEntry.id.desc()) .all() diff --git a/warehouse/legacy/api/json.py b/warehouse/legacy/api/json.py index 78102444e44b..5e44f86b1365 100644 --- a/warehouse/legacy/api/json.py +++ b/warehouse/legacy/api/json.py @@ -195,11 +195,13 @@ def json_release(release, request): context=Release, decorator=_CACHE_DECORATOR, ) -def json_release_slash(project, request): +def json_release_slash(release, request): return HTTPMovedPermanently( # Respond with redirect to url without trailing slash request.route_path( - "legacy.api.json.release", name=project.name, version=project.version + "legacy.api.json.release", + name=release.project.name, + version=release.version, ), headers=_CORS_HEADERS, ) diff --git a/warehouse/legacy/api/simple.py b/warehouse/legacy/api/simple.py index 6626d6cdb567..3fc696b66a1d 100644 --- a/warehouse/legacy/api/simple.py +++ b/warehouse/legacy/api/simple.py @@ -80,16 +80,10 @@ def simple_detail(project, request): files = sorted( request.db.query(File) .options(joinedload(File.release)) - .filter( - File.name == project.name, - File.version.in_( - request.db.query(Release) - .filter(Release.project == project) - .with_entities(Release.version) - ), - ) + .join(Release) + .filter(Release.project == project) .all(), - key=lambda f: (parse(f.version), f.filename), + key=lambda f: (parse(f.release.version), f.filename), ) return {"project": project, "files": files} diff --git a/warehouse/legacy/api/xmlrpc/views.py b/warehouse/legacy/api/xmlrpc/views.py index 248dce5f3ab1..f32f0f3b2317 100644 --- a/warehouse/legacy/api/xmlrpc/views.py +++ b/warehouse/legacy/api/xmlrpc/views.py @@ -491,15 +491,12 @@ def browse(request, classifiers: List[str]): ) releases = ( - request.db.query(Release.name, Release.version) - .join( - release_classifiers_q, - (Release.name == release_classifiers_q.c.name) - & (Release.version == release_classifiers_q.c.version), - ) - .group_by(Release.name, Release.version) + request.db.query(Project.name, Release.version) + .join(Release) + .join(release_classifiers_q, Release.id == release_classifiers_q.c.release_id) + .group_by(Project.name, Release.version) .having(func.count() == len(classifiers)) - .order_by(Release.name, Release.version) + .order_by(Project.name, Release.version) .all() ) diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index 8d2e7c918770..e11363bbf0d0 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -49,16 +49,21 @@ def user_projects(request): ) with_sole_owner = ( - request.db.query(Role.package_id) + request.db.query(Role.project_id) .join(projects_owned) .filter(Role.role_name == "Owner") - .group_by(Role.package_id) - .having(func.count(Role.package_id) == 1) + .group_by(Role.project_id) + .having(func.count(Role.project_id) == 1) .subquery() ) return { - "projects_owned": request.db.query(projects_owned).all(), + "projects_owned": ( + request.db.query(Project) + .join(projects_owned, Project.id == projects_owned.c.id) + .order_by(Project.name) + .all() + ), "projects_sole_owned": ( request.db.query(Project).join(with_sole_owner).order_by(Project.name).all() ), @@ -454,7 +459,7 @@ def _error(message): release_file = ( self.request.db.query(File) .filter( - File.name == self.release.project.name, + File.release == self.release, File.id == self.request.POST.get("file_id"), ) .one() diff --git a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py index 2dccf506c47e..3ac5e8722838 100644 --- a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py +++ b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py @@ -118,8 +118,6 @@ def upgrade(): op.alter_column("release_dependencies", "release_id", nullable=False) op.alter_column("release_classifiers", "release_id", nullable=False) - op.drop_column("roles", "package_name") - op.drop_constraint( "release_classifiers_name_fkey", "release_classifiers", type_="foreignkey" ) @@ -186,6 +184,79 @@ def upgrade(): ondelete="CASCADE", ) + op.drop_index("rel_dep_name_version_kind_idx", table_name="release_dependencies") + op.create_index( + "release_dependencies_release_kind_idx", + "release_dependencies", + ["release_id", "kind"], + ) + + op.drop_index("release_name_created_idx", table_name="releases") + op.create_index( + "release_project_created_idx", + "releases", + ["project_id", sa.text("created DESC")], + ) + + op.drop_index("release_files_name_version_idx", table_name="release_files") + op.drop_index("release_files_version_idx", table_name="release_files") + op.drop_index("release_files_single_sdist", table_name="release_files") + + op.create_index("release_files_release_id_idx", "release_files", ["release_id"]) + op.create_index( + "release_files_single_sdist", + "release_files", + ["release_id", "packagetype"], + unique=True, + postgresql_where=sa.text( + "packagetype = 'sdist' AND allow_multiple_sdist = false" + ), + ) + + op.drop_index("rel_class_name_version_idx", table_name="release_classifiers") + op.drop_index("rel_class_version_id_idx", table_name="release_classifiers") + + op.create_index("rel_class_release_id_idx", "release_classifiers", ["release_id"]) + + op.drop_column("roles", "package_name") + op.drop_column("releases", "name") + op.drop_column("release_files", "name") + op.drop_column("release_files", "version") + op.drop_column("release_classifiers", "name") + op.drop_column("release_classifiers", "version") + op.drop_column("release_dependencies", "name") + op.drop_column("release_dependencies", "version") + + op.execute( + """CREATE OR REPLACE FUNCTION update_release_files_requires_python() + RETURNS TRIGGER AS $$ + BEGIN + IF (TG_TABLE_NAME = 'releases') THEN + UPDATE + release_files + SET + requires_python = releases.requires_python + FROM releases + WHERE + release_files.release_id = releases.id + AND releases.id = NEW.id; + ELSEIF (TG_TABLE_NAME = 'release_files') THEN + UPDATE + release_files + SET + requires_python = releases.requires_python + FROM releases + WHERE + release_files.release_id = releases.id + AND releases.id = NEW.release_id; + END IF; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """ + ) + def downgrade(): raise RuntimeError("Order No. 227 - Ни шагу назад!") diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index fcced6eacd12..56f3ecd05228 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -96,7 +96,7 @@ def __getitem__(self, project): raise KeyError from None -class Project(SitemapMixin, db.ModelBase): +class Project(SitemapMixin, db.Model): __tablename__ = "packages" __table_args__ = ( @@ -108,7 +108,6 @@ class Project(SitemapMixin, db.ModelBase): __repr__ = make_repr("name") - id = Column(UUID(as_uuid=True), primary_key=True, nullable=False) name = Column(Text, nullable=False) normalized_name = orm.column_property(func.normalize_pep426_name(name)) created = Column( @@ -230,7 +229,7 @@ class Dependency(db.Model): __tablename__ = "release_dependencies" __table_args__ = ( - Index("rel_dep_name_version_kind_idx", "name", "version", "kind"), + Index("release_dependencies_release_kind_idx", "release_id", "kind"), ForeignKeyConstraint( ["release_id"], ["releases.id"], onupdate="CASCADE", ondelete="CASCADE" ), @@ -238,8 +237,6 @@ class Dependency(db.Model): __repr__ = make_repr("name", "version", "kind", "specifier") release_id = Column(ForeignKey("releases.id"), nullable=False) - name = Column(Text) - version = Column(Text) kind = Column(Integer) specifier = Column(Text) @@ -254,7 +251,7 @@ def _dependency_relation(kind): ) -class Release(db.ModelBase): +class Release(db.Model): __tablename__ = "releases" @@ -265,17 +262,15 @@ def __table_args__(cls): # noqa ["project_id"], ["packages.id"], onupdate="CASCADE", ondelete="CASCADE" ), Index("release_created_idx", cls.created.desc()), - Index("release_name_created_idx", cls.name, cls.created.desc()), + Index("release_project_created_idx", cls.project_id, cls.created.desc()), Index("release_version_idx", cls.version), ) - __repr__ = make_repr("name", "version") + __repr__ = make_repr("project", "version") __parent__ = dotted_navigator("project") __name__ = dotted_navigator("version") - id = Column(UUID(as_uuid=True), primary_key=True, nullable=False) project_id = Column(ForeignKey("packages.id"), nullable=False) - name = Column(Text, nullable=False) version = Column(Text, nullable=False) canonical_version = Column(Text, nullable=False) is_prerelease = orm.column_property(func.pep440_is_prerelease(version)) @@ -323,7 +318,12 @@ def __table_args__(cls): # noqa passive_deletes=True, ) - dependencies = orm.relationship("Dependency") + dependencies = orm.relationship( + "Dependency", + backref="release", + cascade="all, delete-orphan", + passive_deletes=True, + ) _requires = _dependency_relation(DependencyKind.requires) requires = association_proxy("_requires", "specifier") @@ -353,7 +353,8 @@ def __table_args__(cls): # noqa "User", secondary=lambda: JournalEntry.__table__, primaryjoin=lambda: ( - (JournalEntry.name == orm.foreign(Release.name)) + (JournalEntry.name == orm.foreign(Project.name)) + & (Project.id == Release.project_id) & (JournalEntry.version == orm.foreign(Release.version)) & (JournalEntry.action == "new release") ), @@ -425,12 +426,9 @@ def __table_args__(cls): # noqa ), CheckConstraint("sha256_digest ~* '^[A-F0-9]{64}$'"), CheckConstraint("blake2_256_digest ~* '^[A-F0-9]{64}$'"), - Index("release_files_name_version_idx", "name", "version"), - Index("release_files_version_idx", "version"), Index( "release_files_single_sdist", - "name", - "version", + "release_id", "packagetype", unique=True, postgresql_where=( @@ -441,8 +439,6 @@ def __table_args__(cls): # noqa ) release_id = Column(ForeignKey("releases.id"), nullable=False) - name = Column(Text) - version = Column(Text) python_version = Column(Text) requires_python = Column(Text) packagetype = Column( @@ -498,12 +494,8 @@ class Filename(db.ModelBase): "release_classifiers", db.metadata, Column("release_id", ForeignKey("releases.id"), nullable=False), - Column("name", Text()), - Column("version", Text()), Column("trove_id", Integer(), ForeignKey("trove_classifiers.id")), - Index("rel_class_name_version_idx", "name", "version"), Index("rel_class_trove_id_idx", "trove_id"), - Index("rel_class_version_id_idx", "version"), ) diff --git a/warehouse/packaging/tasks.py b/warehouse/packaging/tasks.py index 998f136c7e9a..b5e3f788d7a3 100644 --- a/warehouse/packaging/tasks.py +++ b/warehouse/packaging/tasks.py @@ -77,10 +77,10 @@ def compute_trending(request): # turn it into the primary key of the Project object and construct a list # of primary key: new zscore, including a default of None if the item isn't # in the result set. - query = request.db.query(Project.name, Project.normalized_name).all() + query = request.db.query(Project.id, Project.normalized_name).all() to_update = [ - {"name": name, "zscore": zscores[normalized_name]} - for name, normalized_name in query + {"id": id, "zscore": zscores[normalized_name]} + for id, normalized_name in query if normalized_name in zscores ] diff --git a/warehouse/search/tasks.py b/warehouse/search/tasks.py index 104c27d97d01..959172ade40e 100644 --- a/warehouse/search/tasks.py +++ b/warehouse/search/tasks.py @@ -16,7 +16,7 @@ from elasticsearch.helpers import parallel_bulk from elasticsearch_dsl import serializer -from sqlalchemy import and_, func +from sqlalchemy import func from sqlalchemy.orm import aliased import certifi import elasticsearch @@ -32,17 +32,17 @@ def _project_docs(db, project_name=None): releases_list = ( - db.query(Release.name, Release.version) + db.query(Release.id) .order_by( - Release.name, + Release.project_id, Release.is_prerelease.nullslast(), Release._pypi_ordering.desc(), ) - .distinct(Release.name) + .distinct(Release.project_id) ) if project_name: - releases_list = releases_list.filter(Release.name == project_name) + releases_list = releases_list.join(Project).filter(Project.name == project_name) releases_list = releases_list.subquery() @@ -50,7 +50,7 @@ def _project_docs(db, project_name=None): all_versions = ( db.query(func.array_agg(r.version)) - .filter(r.name == Release.name) + .filter(r.project_id == Release.project_id) .correlate(Release) .as_scalar() .label("all_versions") @@ -60,8 +60,7 @@ def _project_docs(db, project_name=None): db.query(func.array_agg(Classifier.classifier)) .select_from(release_classifiers) .join(Classifier, Classifier.id == release_classifiers.c.trove_id) - .filter(Release.name == release_classifiers.c.name) - .filter(Release.version == release_classifiers.c.version) + .filter(Release.id == release_classifiers.c.release_id) .correlate(Release) .as_scalar() .label("classifiers") @@ -70,7 +69,6 @@ def _project_docs(db, project_name=None): release_data = ( db.query( Release.description, - Release.name, Release.version.label("latest_version"), all_versions, Release.author, @@ -88,18 +86,11 @@ def _project_docs(db, project_name=None): Project.name, ) .select_from(releases_list) - .join( - Release, - and_( - Release.name == releases_list.c.name, - Release.version == releases_list.c.version, - ), - ) + .join(Release, Release.id == releases_list.c.id) .outerjoin(Release.project) - .order_by(Release.name) ) - for release in windowed_query(release_data, Release.name, 50000): + for release in windowed_query(release_data, Release.project_id, 50000): p = ProjectDocument.from_db(release) p._index = None p.full_clean() diff --git a/warehouse/views.py b/warehouse/views.py index 2dc3fd251ed7..d909334419e7 100644 --- a/warehouse/views.py +++ b/warehouse/views.py @@ -166,10 +166,10 @@ def opensearchxml(request): ], ) def index(request): - project_names = [ + project_ids = [ r[0] for r in ( - request.db.query(Project.name) + request.db.query(Project.id) .order_by(Project.zscore.desc().nullslast(), func.random()) .limit(5) .all() @@ -178,10 +178,10 @@ def index(request): release_a = aliased( Release, request.db.query(Release) - .distinct(Release.name) - .filter(Release.name.in_(project_names)) + .distinct(Release.project_id) + .filter(Release.project_id.in_(project_ids)) .order_by( - Release.name, + Release.project_id, Release.is_prerelease.nullslast(), Release._pypi_ordering.desc(), ) @@ -190,7 +190,7 @@ def index(request): trending_projects = ( request.db.query(release_a) .options(joinedload(release_a.project)) - .order_by(func.array_idx(project_names, release_a.name)) + .order_by(func.array_idx(project_ids, release_a.project_id)) .all() ) @@ -360,8 +360,10 @@ def filter_key(item): def stats(request): total_size_query = request.db.query(func.sum(File.size)).all() top_100_packages = ( - request.db.query(File.name, func.sum(File.size)) - .group_by(File.name) + request.db.query(Project.name, func.sum(File.size)) + .join(Release) + .join(File) + .group_by(Project.name) .order_by(func.sum(File.size).desc()) .limit(100) .all() From a007d1d568df96218fc0c7d91a4bbe1e3707db3f Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Mon, 5 Nov 2018 22:59:39 -0500 Subject: [PATCH 07/13] Update for #5001 --- warehouse/admin/squats.py | 14 ++--- ...6a223_migrate_projects_and_releases_to_.py | 58 ++++++++++++++++++- 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/warehouse/admin/squats.py b/warehouse/admin/squats.py index 6df274a5d32f..5cfe485130a1 100644 --- a/warehouse/admin/squats.py +++ b/warehouse/admin/squats.py @@ -10,7 +10,7 @@ # See the License for the specific language governing permissions and # limitations under the License. -from sqlalchemy import Boolean, Column, DateTime, ForeignKey, Integer, Text +from sqlalchemy import Boolean, Column, DateTime, ForeignKey, Integer from sqlalchemy import orm, sql from warehouse import db @@ -24,12 +24,12 @@ class Squat(db.ModelBase): created = Column( DateTime(timezone=False), nullable=False, server_default=sql.func.now() ) - squatter_name = Column( - Text, ForeignKey("packages.name", onupdate="CASCADE", ondelete="CASCADE") + squatter_id = Column( + ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE") ) - squattee_name = Column( - Text, ForeignKey("packages.name", onupdate="CASCADE", ondelete="CASCADE") + squattee_id = Column( + ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE") ) - squatter = orm.relationship("Project", foreign_keys=[squatter_name], lazy=False) - squattee = orm.relationship("Project", foreign_keys=[squattee_name], lazy=False) + squatter = orm.relationship("Project", foreign_keys=[squatter_id], lazy=False) + squattee = orm.relationship("Project", foreign_keys=[squattee_id], lazy=False) reviewed = Column(Boolean, nullable=False, server_default=sql.false()) diff --git a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py index 3ac5e8722838..34737fdd7fdc 100644 --- a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py +++ b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py @@ -22,7 +22,7 @@ from sqlalchemy.dialects import postgresql revision = "ee5b8f66a223" -down_revision = "e82c3a017d60" +down_revision = "eeb23d9b4d00" def upgrade(): @@ -63,6 +63,14 @@ def upgrade(): "release_classifiers", sa.Column("release_id", postgresql.UUID(as_uuid=True), nullable=True), ) + op.add_column( + "warehouse_admin_squat", + sa.Column("squattee_id", postgresql.UUID(as_uuid=True), nullable=True), + ) + op.add_column( + "warehouse_admin_squat", + sa.Column("squatter_id", postgresql.UUID(as_uuid=True), nullable=True), + ) op.execute( """ UPDATE releases @@ -112,11 +120,29 @@ def upgrade(): AND release_classifiers.version = releases.version """ ) + op.execute( + """ UPDATE warehouse_admin_squat + SET squattee_id = packages.id + FROM packages + WHERE + packages.name = warehouse_admin_squat.squattee_name + """ + ) + op.execute( + """ UPDATE warehouse_admin_squat + SET squatter_id = packages.id + FROM packages + WHERE + packages.name = warehouse_admin_squat.squatter_name + """ + ) op.alter_column("releases", "project_id", nullable=False) op.alter_column("release_files", "release_id", nullable=False) op.alter_column("release_dependencies", "release_id", nullable=False) op.alter_column("release_classifiers", "release_id", nullable=False) + op.alter_column("warehouse_admin_squat", "squattee_id", nullable=False) + op.alter_column("warehouse_admin_squat", "squatter_id", nullable=False) op.drop_constraint( "release_classifiers_name_fkey", "release_classifiers", type_="foreignkey" @@ -126,6 +152,16 @@ def upgrade(): ) op.drop_constraint("release_files_name_fkey", "release_files", type_="foreignkey") op.drop_constraint("releases_name_fkey", "releases", type_="foreignkey") + op.drop_constraint( + "warehouse_admin_squat_squattee_name_fkey", + "warehouse_admin_squat", + type_="foreignkey", + ) + op.drop_constraint( + "warehouse_admin_squat_squatter_name_fkey", + "warehouse_admin_squat", + type_="foreignkey", + ) op.execute("ALTER TABLE packages DROP CONSTRAINT packages_pkey CASCADE") op.create_primary_key(None, "packages", ["id"]) @@ -183,6 +219,24 @@ def upgrade(): onupdate="CASCADE", ondelete="CASCADE", ) + op.create_foreign_key( + None, + "warehouse_admin_squat", + "packages", + ["squattee_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + op.create_foreign_key( + None, + "warehouse_admin_squat", + "packages", + ["squatter_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) op.drop_index("rel_dep_name_version_kind_idx", table_name="release_dependencies") op.create_index( @@ -226,6 +280,8 @@ def upgrade(): op.drop_column("release_classifiers", "version") op.drop_column("release_dependencies", "name") op.drop_column("release_dependencies", "version") + op.drop_column("warehouse_admin_squat", "squattee_name") + op.drop_column("warehouse_admin_squat", "squatter_name") op.execute( """CREATE OR REPLACE FUNCTION update_release_files_requires_python() From e96d101adb7f6f5fbeb6c134a1c47339a88e28bc Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Tue, 6 Nov 2018 02:56:38 -0500 Subject: [PATCH 08/13] Ensure model state matches db state --- warehouse/admin/squats.py | 6 ++-- ...6a223_migrate_projects_and_releases_to_.py | 4 +++ warehouse/packaging/models.py | 32 +++++++++++-------- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/warehouse/admin/squats.py b/warehouse/admin/squats.py index 5cfe485130a1..b1cada82cd57 100644 --- a/warehouse/admin/squats.py +++ b/warehouse/admin/squats.py @@ -25,10 +25,12 @@ class Squat(db.ModelBase): DateTime(timezone=False), nullable=False, server_default=sql.func.now() ) squatter_id = Column( - ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE") + ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, ) squattee_id = Column( - ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE") + ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, ) squatter = orm.relationship("Project", foreign_keys=[squatter_id], lazy=False) squattee = orm.relationship("Project", foreign_keys=[squattee_id], lazy=False) diff --git a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py index 34737fdd7fdc..37df58cc9ebc 100644 --- a/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py +++ b/warehouse/migrations/versions/ee5b8f66a223_migrate_projects_and_releases_to_.py @@ -137,6 +137,10 @@ def upgrade(): """ ) + # We need to delete the old admin roles, which aren't actually used anymore. + op.execute("DELETE FROM roles WHERE role_name = 'Admin'") + + op.alter_column("roles", "project_id", nullable=False) op.alter_column("releases", "project_id", nullable=False) op.alter_column("release_files", "release_id", nullable=False) op.alter_column("release_dependencies", "release_id", nullable=False) diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 56f3ecd05228..869de9cfde1f 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -230,13 +230,13 @@ class Dependency(db.Model): __tablename__ = "release_dependencies" __table_args__ = ( Index("release_dependencies_release_kind_idx", "release_id", "kind"), - ForeignKeyConstraint( - ["release_id"], ["releases.id"], onupdate="CASCADE", ondelete="CASCADE" - ), ) __repr__ = make_repr("name", "version", "kind", "specifier") - release_id = Column(ForeignKey("releases.id"), nullable=False) + release_id = Column( + ForeignKey("releases.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) kind = Column(Integer) specifier = Column(Text) @@ -258,9 +258,6 @@ class Release(db.Model): @declared_attr def __table_args__(cls): # noqa return ( - ForeignKeyConstraint( - ["project_id"], ["packages.id"], onupdate="CASCADE", ondelete="CASCADE" - ), Index("release_created_idx", cls.created.desc()), Index("release_project_created_idx", cls.project_id, cls.created.desc()), Index("release_version_idx", cls.version), @@ -270,7 +267,10 @@ def __table_args__(cls): # noqa __parent__ = dotted_navigator("project") __name__ = dotted_navigator("version") - project_id = Column(ForeignKey("packages.id"), nullable=False) + project_id = Column( + ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) version = Column(Text, nullable=False) canonical_version = Column(Text, nullable=False) is_prerelease = orm.column_property(func.pep440_is_prerelease(version)) @@ -421,9 +421,6 @@ class File(db.Model): @declared_attr def __table_args__(cls): # noqa return ( - ForeignKeyConstraint( - ["release_id"], ["releases.id"], onupdate="CASCADE", ondelete="CASCADE" - ), CheckConstraint("sha256_digest ~* '^[A-F0-9]{64}$'"), CheckConstraint("blake2_256_digest ~* '^[A-F0-9]{64}$'"), Index( @@ -436,9 +433,13 @@ def __table_args__(cls): # noqa & (cls.allow_multiple_sdist == False) # noqa ), ), + Index("release_files_release_id_idx", "release_id"), ) - release_id = Column(ForeignKey("releases.id"), nullable=False) + release_id = Column( + ForeignKey("releases.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) python_version = Column(Text) requires_python = Column(Text) packagetype = Column( @@ -493,9 +494,14 @@ class Filename(db.ModelBase): release_classifiers = Table( "release_classifiers", db.metadata, - Column("release_id", ForeignKey("releases.id"), nullable=False), + Column( + "release_id", + ForeignKey("releases.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ), Column("trove_id", Integer(), ForeignKey("trove_classifiers.id")), Index("rel_class_trove_id_idx", "trove_id"), + Index("rel_class_release_id_idx", "release_id"), ) From cf1bdc24bd5fcc6bbf90e900f47318ee47a254d8 Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Tue, 6 Nov 2018 11:07:05 -0500 Subject: [PATCH 09/13] Switch to using FKs to User.id isntead of User.username --- tests/unit/admin/views/test_projects.py | 2 +- .../templates/admin/projects/detail.html | 6 +- warehouse/admin/views/projects.py | 6 +- warehouse/manage/views.py | 9 ++- ...9dd_move_all_user_fks_to_id_instead_of_.py | 58 +++++++++++++++++++ warehouse/packaging/models.py | 8 +-- 6 files changed, 75 insertions(+), 14 deletions(-) create mode 100644 warehouse/migrations/versions/5538f2d929dd_move_all_user_fks_to_id_instead_of_.py diff --git a/tests/unit/admin/views/test_projects.py b/tests/unit/admin/views/test_projects.py index d3617f685a59..3f6a53ceb1b7 100644 --- a/tests/unit/admin/views/test_projects.py +++ b/tests/unit/admin/views/test_projects.py @@ -567,7 +567,7 @@ def test_delete_role(self, db_request): assert db_request.session.flash.calls == [ pretend.call( - f"Removed '{role.user_name}' as '{role.role_name}' on '{project.name}'", + f"Removed '{role.user.username}' as '{role.role_name}' on '{project.name}'", queue="success", ) ] diff --git a/warehouse/admin/templates/admin/projects/detail.html b/warehouse/admin/templates/admin/projects/detail.html index cb765714b6aa..3343699a4a9d 100644 --- a/warehouse/admin/templates/admin/projects/detail.html +++ b/warehouse/admin/templates/admin/projects/detail.html @@ -86,7 +86,7 @@

Maintainers:

- + @@ -97,9 +97,9 @@
diff --git a/warehouse/admin/views/projects.py b/warehouse/admin/views/projects.py index cd9864a8c931..ffb1fa369509 100644 --- a/warehouse/admin/views/projects.py +++ b/warehouse/admin/views/projects.py @@ -379,7 +379,7 @@ def delete_role(project, request): ) ) - if not confirm or confirm != role.user_name: + if not confirm or confirm != role.user.username: request.session.flash("Confirm the request", queue="error") raise HTTPSeeOther( request.route_path( @@ -388,13 +388,13 @@ def delete_role(project, request): ) request.session.flash( - f"Removed '{role.user_name}' as '{role.role_name}' on '{project.name}'", + f"Removed '{role.user.username}' as '{role.role_name}' on '{project.name}'", queue="success", ) request.db.add( JournalEntry( name=project.name, - action=f"remove {role.role_name} {role.user_name}", + action=f"remove {role.role_name} {role.user.username}", submitted_by=request.user, submitted_from=request.remote_addr, ) diff --git a/warehouse/manage/views.py b/warehouse/manage/views.py index e11363bbf0d0..a3e3b18952b4 100644 --- a/warehouse/manage/views.py +++ b/warehouse/manage/views.py @@ -615,6 +615,7 @@ def change_project_role(project, request, _form_class=ChangeRoleForm): # TODO: This branch should be removed when fixing GH-2745. roles = ( request.db.query(Role) + .join(User) .filter( Role.id.in_(role_ids), Role.project == project, @@ -634,7 +635,7 @@ def change_project_role(project, request, _form_class=ChangeRoleForm): request.db.add( JournalEntry( name=project.name, - action=f"remove {role.role_name} {role.user_name}", + action=f"remove {role.role_name} {role.user.username}", submitted_by=request.user, submitted_from=request.remote_addr, ) @@ -645,6 +646,7 @@ def change_project_role(project, request, _form_class=ChangeRoleForm): try: role = ( request.db.query(Role) + .join(User) .filter( Role.id == request.POST.get("role_id"), Role.project == project ) @@ -659,7 +661,7 @@ def change_project_role(project, request, _form_class=ChangeRoleForm): JournalEntry( name=project.name, action="change {} {} to {}".format( - role.role_name, role.user_name, form.role_name.data + role.role_name, role.user.username, form.role_name.data ), submitted_by=request.user, submitted_from=request.remote_addr, @@ -688,6 +690,7 @@ def delete_project_role(project, request): roles = ( request.db.query(Role) + .join(User) .filter(Role.id.in_(request.POST.getall("role_id")), Role.project == project) .all() ) @@ -705,7 +708,7 @@ def delete_project_role(project, request): request.db.add( JournalEntry( name=project.name, - action=f"remove {role.role_name} {role.user_name}", + action=f"remove {role.role_name} {role.user.username}", submitted_by=request.user, submitted_from=request.remote_addr, ) diff --git a/warehouse/migrations/versions/5538f2d929dd_move_all_user_fks_to_id_instead_of_.py b/warehouse/migrations/versions/5538f2d929dd_move_all_user_fks_to_id_instead_of_.py new file mode 100644 index 000000000000..b88a9f6df202 --- /dev/null +++ b/warehouse/migrations/versions/5538f2d929dd_move_all_user_fks_to_id_instead_of_.py @@ -0,0 +1,58 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Move All User FKs to id instead of username + +Revision ID: 5538f2d929dd +Revises: ee5b8f66a223 +Create Date: 2018-11-06 08:16:12.099174 +""" + +from alembic import op +import sqlalchemy as sa + +from sqlalchemy.dialects import postgresql + + +revision = "5538f2d929dd" +down_revision = "ee5b8f66a223" + + +def upgrade(): + op.add_column( + "roles", sa.Column("user_id", postgresql.UUID(as_uuid=True), nullable=True) + ) + op.execute( + """ UPDATE roles + SET user_id = accounts_user.id + FROM accounts_user + WHERE roles.user_name = accounts_user.username + """ + ) + op.alter_column("roles", "user_id", nullable=False) + op.drop_constraint("roles_user_name_fkey", "roles", type_="foreignkey") + op.create_foreign_key( + None, + "roles", + "accounts_user", + ["user_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + op.drop_index("roles_user_name_idx", table_name="roles") + op.create_index("roles_user_id_idx", "roles", ["user_id"]) + op.drop_column("roles", "user_name") + + +def downgrade(): + raise RuntimeError("Order No. 227 - Ни шагу назад!") diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 869de9cfde1f..8c6f3966f85a 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -53,14 +53,14 @@ class Role(db.Model): __tablename__ = "roles" - __table_args__ = (Index("roles_user_name_idx", "user_name"),) + __table_args__ = (Index("roles_user_id_idx", "user_id"),) __repr__ = make_repr("role_name", "user_name", "package_name") role_name = Column(Text) - user_name = Column( - CIText, - ForeignKey("accounts_user.username", onupdate="CASCADE", ondelete="CASCADE"), + user_id = Column( + ForeignKey("accounts_user.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, ) project_id = Column( ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), From d9696aa46b2d1c9e03cefa3a30d74012fd5f7bc7 Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Tue, 6 Nov 2018 13:36:23 -0500 Subject: [PATCH 10/13] Fix test --- tests/unit/search/test_tasks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/search/test_tasks.py b/tests/unit/search/test_tasks.py index 6cc76a4ee5d6..b89365799dc5 100644 --- a/tests/unit/search/test_tasks.py +++ b/tests/unit/search/test_tasks.py @@ -57,7 +57,7 @@ def test_project_docs(db_session): "latest_version": first(prs, key=lambda r: not r.is_prerelease).version, }, } - for p, prs in sorted(releases.items(), key=lambda x: x[0].name.lower()) + for p, prs in sorted(releases.items(), key=lambda x: x[0].id) ] From 96227ff920bd08f95869308a396663292c6fa005 Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Tue, 6 Nov 2018 13:44:45 -0500 Subject: [PATCH 11/13] Fix linting --- tests/unit/admin/views/test_projects.py | 3 ++- warehouse/packaging/models.py | 1 - 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit/admin/views/test_projects.py b/tests/unit/admin/views/test_projects.py index 3f6a53ceb1b7..5279ac8d4dce 100644 --- a/tests/unit/admin/views/test_projects.py +++ b/tests/unit/admin/views/test_projects.py @@ -567,7 +567,8 @@ def test_delete_role(self, db_request): assert db_request.session.flash.calls == [ pretend.call( - f"Removed '{role.user.username}' as '{role.role_name}' on '{project.name}'", + f"Removed '{role.user.username}' as '{role.role_name}' " + f"on '{project.name}'", queue="success", ) ] diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 8c6f3966f85a..083d45dd655c 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -25,7 +25,6 @@ Column, Enum, ForeignKey, - ForeignKeyConstraint, Index, Boolean, DateTime, From c42ccc976b0262cdc3102ac6b1f605d80de6c206 Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Tue, 6 Nov 2018 14:22:12 -0500 Subject: [PATCH 12/13] Add Release.uploader as a real FK (#5015) --- .../templates/admin/projects/detail.html | 6 +- .../admin/projects/release_detail.html | 6 +- .../admin/projects/releases_list.html | 6 +- warehouse/forklift/legacy.py | 1 + ...2a92c1017_add_uploader_field_to_release.py | 71 +++++++++++++++++++ warehouse/packaging/models.py | 26 ++----- 6 files changed, 92 insertions(+), 24 deletions(-) create mode 100644 warehouse/migrations/versions/e612a92c1017_add_uploader_field_to_release.py diff --git a/warehouse/admin/templates/admin/projects/detail.html b/warehouse/admin/templates/admin/projects/detail.html index 3343699a4a9d..387c24c61b00 100644 --- a/warehouse/admin/templates/admin/projects/detail.html +++ b/warehouse/admin/templates/admin/projects/detail.html @@ -148,7 +148,11 @@

Releases:

{{ release.name }}-{{ release.version }} {{ release.created }} - {{ release.uploader.username }} + + {% if release.uploader %} + {{ release.uploader.username }} + {% endif %} + {{ release.author_email }} {% endfor %} diff --git a/warehouse/admin/templates/admin/projects/release_detail.html b/warehouse/admin/templates/admin/projects/release_detail.html index adb322a5dc71..64a86587cc82 100644 --- a/warehouse/admin/templates/admin/projects/release_detail.html +++ b/warehouse/admin/templates/admin/projects/release_detail.html @@ -41,7 +41,11 @@

Details

Uploader - {{ release.uploader.username }} + + {% if release.uploader %} + {{ release.uploader.username }} + {% endif %} + Created via diff --git a/warehouse/admin/templates/admin/projects/releases_list.html b/warehouse/admin/templates/admin/projects/releases_list.html index e932d3610339..93ef717a53db 100644 --- a/warehouse/admin/templates/admin/projects/releases_list.html +++ b/warehouse/admin/templates/admin/projects/releases_list.html @@ -51,7 +51,11 @@ {{ release.name }}-{{ release.version }} {{ release.created }} - {{ release.uploader.username }} + + {% if release.uploader %} + {{ release.uploader.username }} + {% endif %} + {{ release.author_email }} {% endfor %} diff --git a/warehouse/forklift/legacy.py b/warehouse/forklift/legacy.py index ace40eb6d268..6e69d66fe8ec 100644 --- a/warehouse/forklift/legacy.py +++ b/warehouse/forklift/legacy.py @@ -1018,6 +1018,7 @@ def file_upload(request): "requires_python", } }, + uploader=request.user, uploaded_via=request.user_agent, ) request.db.add(release) diff --git a/warehouse/migrations/versions/e612a92c1017_add_uploader_field_to_release.py b/warehouse/migrations/versions/e612a92c1017_add_uploader_field_to_release.py new file mode 100644 index 000000000000..4febcbdc6aaa --- /dev/null +++ b/warehouse/migrations/versions/e612a92c1017_add_uploader_field_to_release.py @@ -0,0 +1,71 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Add uploader field to Release + +Revision ID: e612a92c1017 +Revises: 5538f2d929dd +Create Date: 2018-11-06 16:22:01.484362 +""" + +from alembic import op +import sqlalchemy as sa + +from sqlalchemy.dialects import postgresql + + +revision = "e612a92c1017" +down_revision = "5538f2d929dd" + + +def upgrade(): + op.add_column( + "releases", + sa.Column("uploader_id", postgresql.UUID(as_uuid=True), nullable=True), + ) + op.execute( + """ + UPDATE releases + SET uploader_id = s.user_id + FROM ( + SELECT accounts_user.id as user_id, + packages.id as project_id, + releases.version as release_version, + ROW_NUMBER() OVER ( + PARTITION BY journals.name, journals.version + ORDER BY journals.id DESC + ) as rn + FROM accounts_user, packages, journals, releases + WHERE journals.name = packages.name + AND journals.version = releases.version + AND journals.action = 'new release' + AND accounts_user.username = journals.submitted_by + ) s + WHERE releases.project_id = s.project_id + AND releases.version = s.release_version + AND s.rn = 1 + """ + ) + op.create_foreign_key( + None, + "releases", + "accounts_user", + ["uploader_id"], + ["id"], + onupdate="CASCADE", + ondelete="SET NULL", + ) + op.create_index("ix_releases_uploader_id", "releases", ["uploader_id"]) + + +def downgrade(): + raise RuntimeError("Order No. 227 - Ни шагу назад!") diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 083d45dd655c..4f140fa50a73 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -348,28 +348,12 @@ def __table_args__(cls): # noqa _project_urls = _dependency_relation(DependencyKind.project_url) project_urls = association_proxy("_project_urls", "specifier") - uploader = orm.relationship( - "User", - secondary=lambda: JournalEntry.__table__, - primaryjoin=lambda: ( - (JournalEntry.name == orm.foreign(Project.name)) - & (Project.id == Release.project_id) - & (JournalEntry.version == orm.foreign(Release.version)) - & (JournalEntry.action == "new release") - ), - secondaryjoin=lambda: ( - (User.username == orm.foreign(JournalEntry._submitted_by)) - ), - order_by=lambda: JournalEntry.id.desc(), - # TODO: We have uselist=False here which raises a warning because - # multiple items were returned. This should only be temporary because - # we should add a nullable FK to JournalEntry so we don't need to rely - # on ordering and implicitly selecting the first object to make this - # happen, - uselist=False, - viewonly=True, + uploader_id = Column( + ForeignKey("accounts_user.id", onupdate="CASCADE", ondelete="SET NULL"), + nullable=True, + index=True, ) - + uploader = orm.relationship(User) uploaded_via = Column(Text) @property From 5139c256315f6fd92cc8a39837a928ec3e20a3e7 Mon Sep 17 00:00:00 2001 From: Donald Stufft Date: Tue, 6 Nov 2018 14:52:32 -0500 Subject: [PATCH 13/13] Rename a number of tables to better fit in current scheme (#5016) --- warehouse/accounts/models.py | 14 +- warehouse/admin/flags.py | 2 +- warehouse/admin/squats.py | 6 +- .../versions/06bfbc92f67d_rename_tables.py | 161 ++++++++++++++++++ warehouse/packaging/models.py | 17 +- 5 files changed, 180 insertions(+), 20 deletions(-) create mode 100644 warehouse/migrations/versions/06bfbc92f67d_rename_tables.py diff --git a/warehouse/accounts/models.py b/warehouse/accounts/models.py index 8fb2badf9719..5d27caf80207 100644 --- a/warehouse/accounts/models.py +++ b/warehouse/accounts/models.py @@ -53,12 +53,12 @@ class DisableReason(enum.Enum): class User(SitemapMixin, db.Model): - __tablename__ = "accounts_user" + __tablename__ = "users" __table_args__ = ( - CheckConstraint("length(username) <= 50", name="packages_valid_name"), + CheckConstraint("length(username) <= 50", name="users_valid_username_length"), CheckConstraint( "username ~* '^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$'", - name="accounts_user_valid_username", + name="users_valid_username", ), ) @@ -110,16 +110,16 @@ class UnverifyReasons(enum.Enum): class Email(db.ModelBase): - __tablename__ = "accounts_email" + __tablename__ = "user_emails" __table_args__ = ( - UniqueConstraint("email", name="accounts_email_email_key"), - Index("accounts_email_user_id", "user_id"), + UniqueConstraint("email", name="user_emails_email_key"), + Index("user_emails_user_id", "user_id"), ) id = Column(Integer, primary_key=True, nullable=False) user_id = Column( UUID(as_uuid=True), - ForeignKey("accounts_user.id", deferrable=True, initially="DEFERRED"), + ForeignKey("users.id", deferrable=True, initially="DEFERRED"), nullable=False, ) email = Column(String(length=254), nullable=False) diff --git a/warehouse/admin/flags.py b/warehouse/admin/flags.py index ae74b79bce51..7b01a5159c06 100644 --- a/warehouse/admin/flags.py +++ b/warehouse/admin/flags.py @@ -17,7 +17,7 @@ class AdminFlag(db.ModelBase): - __tablename__ = "warehouse_admin_flag" + __tablename__ = "admin_flags" id = Column(Text, primary_key=True, nullable=False) description = Column(Text, nullable=False) diff --git a/warehouse/admin/squats.py b/warehouse/admin/squats.py index b1cada82cd57..ccb1d68562f4 100644 --- a/warehouse/admin/squats.py +++ b/warehouse/admin/squats.py @@ -18,18 +18,18 @@ class Squat(db.ModelBase): - __tablename__ = "warehouse_admin_squat" + __tablename__ = "admin_squats" id = Column(Integer, primary_key=True, nullable=False) created = Column( DateTime(timezone=False), nullable=False, server_default=sql.func.now() ) squatter_id = Column( - ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, ) squattee_id = Column( - ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, ) squatter = orm.relationship("Project", foreign_keys=[squatter_id], lazy=False) diff --git a/warehouse/migrations/versions/06bfbc92f67d_rename_tables.py b/warehouse/migrations/versions/06bfbc92f67d_rename_tables.py new file mode 100644 index 000000000000..24f4eba3f453 --- /dev/null +++ b/warehouse/migrations/versions/06bfbc92f67d_rename_tables.py @@ -0,0 +1,161 @@ +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +""" +Rename tables + +Revision ID: 06bfbc92f67d +Revises: eeb23d9b4d00 +Create Date: 2018-11-06 04:36:58.531272 +""" + +from alembic import op + + +revision = "06bfbc92f67d" +down_revision = "e612a92c1017" + + +def upgrade(): + # The new verbiage in Warehouse is to call these things packages, but this table + # name was inherited from legacy PyPI. + op.rename_table("packages", "projects") + op.execute("ALTER TABLE projects RENAME CONSTRAINT packages_pkey TO projects_pkey") + op.execute( + """ + ALTER TABLE projects + RENAME CONSTRAINT packages_valid_name + TO projects_valid_name + """ + ) + op.execute( + """ CREATE OR REPLACE FUNCTION maintain_project_last_serial() + RETURNS TRIGGER AS $$ + DECLARE + targeted_name text; + BEGIN + IF TG_OP = 'INSERT' THEN + targeted_name := NEW.name; + ELSEIF TG_OP = 'UPDATE' THEN + targeted_name := NEW.name; + ELSIF TG_OP = 'DELETE' THEN + targeted_name := OLD.name; + END IF; + + UPDATE projects + SET last_serial = j.last_serial + FROM ( + SELECT max(id) as last_serial + FROM journals + WHERE journals.name = targeted_name + ) as j + WHERE projects.name = targeted_name; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """ + ) + op.execute( + "UPDATE row_counts SET table_name = 'projects' WHERE table_name = 'packages'" + ) + + # We took the name of these tables from a failed Django port, the new names are + # cleaner and fit the overall "theme" of our table names better. + op.rename_table("accounts_user", "users") + op.execute("ALTER TABLE users RENAME CONSTRAINT accounts_user_pkey TO users_pkey") + op.execute( + """ + ALTER TABLE users + RENAME CONSTRAINT accounts_user_username_key + TO users_username_key + """ + ) + op.execute( + """ + ALTER TABLE users + RENAME CONSTRAINT accounts_user_valid_username + TO users_valid_username + """ + ) + op.execute( + """ + ALTER TABLE users + RENAME CONSTRAINT packages_valid_name + TO users_valid_username_length + """ + ) + op.execute( + "UPDATE row_counts SET table_name = 'users' WHERE table_name = 'accounts_user'" + ) + + op.rename_table("accounts_email", "user_emails") + op.execute( + """ + ALTER TABLE user_emails + RENAME CONSTRAINT accounts_email_pkey + TO user_emails_pkey + """ + ) + op.execute( + """ + ALTER TABLE user_emails + RENAME CONSTRAINT accounts_email_email_key + TO user_emails_email_key + """ + ) + op.execute( + """ + ALTER TABLE user_emails + RENAME CONSTRAINT accounts_email_user_id_fkey + TO user_emails_user_id_fkey + """ + ) + op.execute("ALTER INDEX accounts_email_user_id RENAME TO user_emails_user_id") + + # While the admin prefix on these tables is useful to let us know they are specific + # to the admins, the warehouse prefix is not. All of these tables in this database + # are for Warehouse. + op.rename_table("warehouse_admin_flag", "admin_flags") + op.execute( + """ + ALTER TABLE admin_flags + RENAME CONSTRAINT warehouse_admin_flag_pkey + TO admin_flags_pkey + """ + ) + + op.rename_table("warehouse_admin_squat", "admin_squats") + op.execute( + """ + ALTER TABLE admin_squats + RENAME CONSTRAINT warehouse_admin_squat_pkey + TO admin_squats_pkey + """ + ) + op.execute( + """ + ALTER TABLE admin_squats + RENAME CONSTRAINT warehouse_admin_squat_squattee_id_fkey + TO admin_squats_squattee_id_fkey + """ + ) + op.execute( + """ + ALTER TABLE admin_squats + RENAME CONSTRAINT warehouse_admin_squat_squatter_id_fkey + TO admin_squats_squatter_id_fkey + """ + ) + + +def downgrade(): + raise RuntimeError("Order No. 227 - Ни шагу назад!") diff --git a/warehouse/packaging/models.py b/warehouse/packaging/models.py index 4f140fa50a73..66e47861d796 100644 --- a/warehouse/packaging/models.py +++ b/warehouse/packaging/models.py @@ -58,11 +58,10 @@ class Role(db.Model): role_name = Column(Text) user_id = Column( - ForeignKey("accounts_user.id", onupdate="CASCADE", ondelete="CASCADE"), - nullable=False, + ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False ) project_id = Column( - ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, ) @@ -97,11 +96,11 @@ def __getitem__(self, project): class Project(SitemapMixin, db.Model): - __tablename__ = "packages" + __tablename__ = "projects" __table_args__ = ( CheckConstraint( "name ~* '^([A-Z0-9]|[A-Z0-9][A-Z0-9._-]*[A-Z0-9])$'::text", - name="packages_valid_name", + name="projects_valid_name", ), ) @@ -267,7 +266,7 @@ def __table_args__(cls): # noqa __name__ = dotted_navigator("version") project_id = Column( - ForeignKey("packages.id", onupdate="CASCADE", ondelete="CASCADE"), + ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, ) version = Column(Text, nullable=False) @@ -349,7 +348,7 @@ def __table_args__(cls): # noqa project_urls = association_proxy("_project_urls", "specifier") uploader_id = Column( - ForeignKey("accounts_user.id", onupdate="CASCADE", ondelete="SET NULL"), + ForeignKey("users.id", onupdate="CASCADE", ondelete="SET NULL"), nullable=True, index=True, ) @@ -509,7 +508,7 @@ def __table_args__(cls): # noqa DateTime(timezone=False), nullable=False, server_default=sql.func.now() ) _submitted_by = Column( - "submitted_by", CIText, ForeignKey("accounts_user.username", onupdate="CASCADE") + "submitted_by", CIText, ForeignKey("users.username", onupdate="CASCADE") ) submitted_by = orm.relationship(User) submitted_from = Column(Text) @@ -532,7 +531,7 @@ class BlacklistedProject(db.Model): ) name = Column(Text, unique=True, nullable=False) _blacklisted_by = Column( - "blacklisted_by", UUID(as_uuid=True), ForeignKey("accounts_user.id"), index=True + "blacklisted_by", UUID(as_uuid=True), ForeignKey("users.id"), index=True ) blacklisted_by = orm.relationship(User) comment = Column(Text, nullable=False, server_default="")