Skip to content

Commit 6cb8bf9

Browse files
ewdurbinewdurbin
authored
Major Database Refactoring (#4958)
* Migrate to UUID Primary Key for Project and Release models * fixup factories for Project and Release, sync Role model to migration * make reformat * fix at least one test * Fix fallout of Role.package_name removal * Remove Denormalized name and version fields from all models (#4974) * Update for #5001 * Ensure model state matches db state * Switch to using FKs to User.id isntead of User.username * Fix test * Fix linting * Add Release.uploader as a real FK (#5015) * Rename a number of tables to better fit in current scheme (#5016)
1 parent b94c6a5 commit 6cb8bf9

31 files changed

+869
-259
lines changed

‎tests/common/db/packaging.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import datetime
1414
import hashlib
15+
import uuid
1516

1617
import factory
1718
import factory.fuzzy
@@ -36,14 +37,15 @@ class ProjectFactory(WarehouseFactory):
3637
class Meta:
3738
model = Project
3839

40+
id = factory.LazyFunction(uuid.uuid4)
3941
name = factory.fuzzy.FuzzyText(length=12)
4042

4143

4244
class ReleaseFactory(WarehouseFactory):
4345
class Meta:
4446
model = Release
4547

46-
name = factory.LazyAttribute(lambda o: o.project.name)
48+
id = factory.LazyFunction(uuid.uuid4)
4749
project = factory.SubFactory(ProjectFactory)
4850
version = factory.Sequence(lambda n: str(n) + ".0")
4951
canonical_version = factory.LazyAttribute(
@@ -58,7 +60,6 @@ class FileFactory(WarehouseFactory):
5860
class Meta:
5961
model = File
6062

61-
name = factory.LazyAttribute(lambda o: o.release.name)
6263
release = factory.SubFactory(ReleaseFactory)
6364
python_version = "source"
6465
md5_digest = factory.LazyAttribute(
@@ -96,8 +97,7 @@ class DependencyFactory(WarehouseFactory):
9697
class Meta:
9798
model = Dependency
9899

99-
name = factory.fuzzy.FuzzyText(length=12)
100-
version = factory.Sequence(lambda n: str(n) + ".0")
100+
release = factory.SubFactory(ReleaseFactory)
101101
kind = factory.fuzzy.FuzzyChoice(int(kind) for kind in DependencyKind)
102102
specifier = factory.fuzzy.FuzzyText(length=12)
103103

‎tests/unit/admin/views/test_blacklist.py

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,7 @@ def test_adds_blacklist_with_deletes(self, db_request):
211211

212212
project = ProjectFactory.create(name="foo")
213213
release = ReleaseFactory.create(project=project)
214-
FileFactory.create(
215-
name=project.name, version=release.version, filename="who cares"
216-
)
214+
FileFactory.create(release=release, filename="who cares")
217215
RoleFactory.create(project=project, user=db_request.user)
218216

219217
views.add_blacklist(db_request)

‎tests/unit/admin/views/test_projects.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -567,7 +567,8 @@ def test_delete_role(self, db_request):
567567

568568
assert db_request.session.flash.calls == [
569569
pretend.call(
570-
f"Removed '{role.user_name}' as '{role.role_name}' on '{project.name}'",
570+
f"Removed '{role.user.username}' as '{role.role_name}' "
571+
f"on '{project.name}'",
571572
queue="success",
572573
)
573574
]

‎tests/unit/legacy/api/test_json.py

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,7 @@ def test_detail_renders(self, pyramid_config, db_request, db_session):
194194
for urlspec in project_urls:
195195
db_session.add(
196196
Dependency(
197-
name=releases[3].project.name,
198-
version="3.0",
197+
release=releases[3],
199198
kind=DependencyKind.project_url.value,
200199
specifier=urlspec,
201200
)
@@ -460,7 +459,9 @@ def test_normalizing_redirects(self, db_request):
460459
assert isinstance(resp, HTTPMovedPermanently)
461460
assert db_request.route_path.calls == [
462461
pretend.call(
463-
"legacy.api.json.release", name=release.name, version=release.version
462+
"legacy.api.json.release",
463+
name=release.project.name,
464+
version=release.version,
464465
)
465466
]
466467
assert resp.headers["Location"] == "/project/the-redirect"

‎tests/unit/legacy/api/xmlrpc/test_xmlrpc.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,7 @@ def test_browse(db_request):
785785
expected_release._classifiers = classifiers
786786

787787
assert set(xmlrpc.browse(db_request, ["Environment :: Other Environment"])) == {
788-
(r.name, r.version) for r in releases
788+
(r.project.name, r.version) for r in releases
789789
}
790790
assert set(
791791
xmlrpc.browse(
@@ -795,7 +795,7 @@ def test_browse(db_request):
795795
"Development Status :: 5 - Production/Stable",
796796
],
797797
)
798-
) == {(expected_release.name, expected_release.version)}
798+
) == {(expected_release.project.name, expected_release.version)}
799799
assert set(
800800
xmlrpc.browse(
801801
db_request,
@@ -805,7 +805,7 @@ def test_browse(db_request):
805805
"Programming Language :: Python",
806806
],
807807
)
808-
) == {(expected_release.name, expected_release.version)}
808+
) == {(expected_release.project.name, expected_release.version)}
809809
assert set(
810810
xmlrpc.browse(
811811
db_request,
@@ -814,7 +814,7 @@ def test_browse(db_request):
814814
"Programming Language :: Python",
815815
],
816816
)
817-
) == {(expected_release.name, expected_release.version)}
817+
) == {(expected_release.project.name, expected_release.version)}
818818

819819

820820
def test_multicall(pyramid_request):

‎tests/unit/manage/test_views.py

Lines changed: 73 additions & 71 deletions
Original file line numberDiff line numberDiff line change
@@ -22,14 +22,15 @@
2222

2323
from warehouse.manage import views
2424
from warehouse.accounts.interfaces import IUserService, IPasswordBreachedService
25-
from warehouse.packaging.models import JournalEntry, Project, Role, User
25+
from warehouse.packaging.models import JournalEntry, Project, File, Role, User
2626
from warehouse.utils.project import remove_documentation
2727

2828
from ...common.db.accounts import EmailFactory
2929
from ...common.db.packaging import (
3030
JournalEntryFactory,
3131
ProjectFactory,
3232
ReleaseFactory,
33+
FileFactory,
3334
RoleFactory,
3435
UserFactory,
3536
)
@@ -977,53 +978,51 @@ def test_delete_project_release_bad_confirm(self):
977978
)
978979
]
979980

980-
def test_delete_project_release_file(self, monkeypatch):
981-
release_file = pretend.stub(filename="foo-bar.tar.gz", id=str(uuid.uuid4()))
982-
release = pretend.stub(version="1.2.3", project=pretend.stub(name="foobar"))
983-
request = pretend.stub(
984-
POST={
985-
"confirm_project_name": release.project.name,
986-
"file_id": release_file.id,
987-
},
988-
method="POST",
989-
db=pretend.stub(
990-
delete=pretend.call_recorder(lambda a: None),
991-
add=pretend.call_recorder(lambda a: None),
992-
query=lambda a: pretend.stub(
993-
filter=lambda *a: pretend.stub(one=lambda: release_file)
994-
),
995-
),
996-
route_path=pretend.call_recorder(lambda *a, **kw: "/the-redirect"),
997-
session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
998-
user=pretend.stub(),
999-
remote_addr=pretend.stub(),
981+
def test_delete_project_release_file(self, db_request):
982+
user = UserFactory.create()
983+
984+
project = ProjectFactory.create(name="foobar")
985+
release = ReleaseFactory.create(project=project)
986+
release_file = FileFactory.create(
987+
release=release, filename=f"foobar-{release.version}.tar.gz"
1000988
)
1001-
journal_obj = pretend.stub()
1002-
journal_cls = pretend.call_recorder(lambda **kw: journal_obj)
1003-
monkeypatch.setattr(views, "JournalEntry", journal_cls)
1004989

1005-
view = views.ManageProjectRelease(release, request)
990+
db_request.POST = {
991+
"confirm_project_name": release.project.name,
992+
"file_id": release_file.id,
993+
}
994+
db_request.method = ("POST",)
995+
db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect")
996+
db_request.session = pretend.stub(
997+
flash=pretend.call_recorder(lambda *a, **kw: None)
998+
)
999+
db_request.user = user
1000+
db_request.remote_addr = "1.2.3.4"
1001+
1002+
view = views.ManageProjectRelease(release, db_request)
10061003

10071004
result = view.delete_project_release_file()
10081005

10091006
assert isinstance(result, HTTPSeeOther)
10101007
assert result.headers["Location"] == "/the-redirect"
10111008

1012-
assert request.session.flash.calls == [
1009+
assert db_request.session.flash.calls == [
10131010
pretend.call(f"Deleted file {release_file.filename!r}", queue="success")
10141011
]
1015-
assert request.db.delete.calls == [pretend.call(release_file)]
1016-
assert request.db.add.calls == [pretend.call(journal_obj)]
1017-
assert journal_cls.calls == [
1018-
pretend.call(
1019-
name=release.project.name,
1020-
action=f"remove file {release_file.filename}",
1012+
1013+
assert db_request.db.query(File).filter_by(id=release_file.id).first() is None
1014+
assert (
1015+
db_request.db.query(JournalEntry)
1016+
.filter_by(
1017+
name=project.name,
10211018
version=release.version,
1022-
submitted_by=request.user,
1023-
submitted_from=request.remote_addr,
1019+
action=f"remove file {release_file.filename}",
1020+
submitted_by=user,
1021+
submitted_from="1.2.3.4",
10241022
)
1025-
]
1026-
assert request.route_path.calls == [
1023+
.one()
1024+
)
1025+
assert db_request.route_path.calls == [
10271026
pretend.call(
10281027
"manage.project.release",
10291028
project_name=release.project.name,
@@ -1059,74 +1058,77 @@ def test_delete_project_release_file_no_confirm(self):
10591058
)
10601059
]
10611060

1062-
def test_delete_project_release_file_not_found(self):
1063-
release = pretend.stub(version="1.2.3", project=pretend.stub(name="foobar"))
1061+
def test_delete_project_release_file_not_found(self, db_request):
1062+
project = ProjectFactory.create(name="foobar")
1063+
release = ReleaseFactory.create(project=project)
10641064

10651065
def no_result_found():
10661066
raise NoResultFound
10671067

1068-
request = pretend.stub(
1069-
POST={"confirm_project_name": "whatever"},
1070-
method="POST",
1071-
db=pretend.stub(
1072-
delete=pretend.call_recorder(lambda a: None),
1073-
query=lambda a: pretend.stub(
1074-
filter=lambda *a: pretend.stub(one=no_result_found)
1075-
),
1068+
db_request.POST = {"confirm_project_name": "whatever"}
1069+
db_request.method = "POST"
1070+
db_request.db = pretend.stub(
1071+
delete=pretend.call_recorder(lambda a: None),
1072+
query=lambda a: pretend.stub(
1073+
filter=lambda *a: pretend.stub(one=no_result_found)
10761074
),
1077-
route_path=pretend.call_recorder(lambda *a, **kw: "/the-redirect"),
1078-
session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
10791075
)
1080-
view = views.ManageProjectRelease(release, request)
1076+
db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect")
1077+
db_request.session = pretend.stub(
1078+
flash=pretend.call_recorder(lambda *a, **kw: None)
1079+
)
1080+
1081+
view = views.ManageProjectRelease(release, db_request)
10811082

10821083
result = view.delete_project_release_file()
10831084

10841085
assert isinstance(result, HTTPSeeOther)
10851086
assert result.headers["Location"] == "/the-redirect"
10861087

1087-
assert request.db.delete.calls == []
1088-
assert request.session.flash.calls == [
1088+
assert db_request.db.delete.calls == []
1089+
assert db_request.session.flash.calls == [
10891090
pretend.call("Could not find file", queue="error")
10901091
]
1091-
assert request.route_path.calls == [
1092+
assert db_request.route_path.calls == [
10921093
pretend.call(
10931094
"manage.project.release",
10941095
project_name=release.project.name,
10951096
version=release.version,
10961097
)
10971098
]
10981099

1099-
def test_delete_project_release_file_bad_confirm(self):
1100-
release_file = pretend.stub(filename="foo-bar.tar.gz", id=str(uuid.uuid4()))
1101-
release = pretend.stub(version="1.2.3", project=pretend.stub(name="foobar"))
1102-
request = pretend.stub(
1103-
POST={"confirm_project_name": "invalid"},
1104-
method="POST",
1105-
db=pretend.stub(
1106-
delete=pretend.call_recorder(lambda a: None),
1107-
query=lambda a: pretend.stub(
1108-
filter=lambda *a: pretend.stub(one=lambda: release_file)
1109-
),
1110-
),
1111-
route_path=pretend.call_recorder(lambda *a, **kw: "/the-redirect"),
1112-
session=pretend.stub(flash=pretend.call_recorder(lambda *a, **kw: None)),
1100+
def test_delete_project_release_file_bad_confirm(self, db_request):
1101+
project = ProjectFactory.create(name="foobar")
1102+
release = ReleaseFactory.create(project=project, version="1.2.3")
1103+
release_file = FileFactory.create(
1104+
release=release, filename="foobar-1.2.3.tar.gz"
11131105
)
1114-
view = views.ManageProjectRelease(release, request)
1106+
1107+
db_request.POST = {
1108+
"confirm_project_name": "invalid",
1109+
"file_id": str(release_file.id),
1110+
}
1111+
db_request.method = "POST"
1112+
db_request.route_path = pretend.call_recorder(lambda *a, **kw: "/the-redirect")
1113+
db_request.session = pretend.stub(
1114+
flash=pretend.call_recorder(lambda *a, **kw: None)
1115+
)
1116+
1117+
view = views.ManageProjectRelease(release, db_request)
11151118

11161119
result = view.delete_project_release_file()
11171120

11181121
assert isinstance(result, HTTPSeeOther)
11191122
assert result.headers["Location"] == "/the-redirect"
1120-
1121-
assert request.db.delete.calls == []
1122-
assert request.session.flash.calls == [
1123+
assert db_request.db.query(File).filter_by(id=release_file.id).one()
1124+
assert db_request.session.flash.calls == [
11231125
pretend.call(
11241126
"Could not delete file - "
11251127
+ f"'invalid' is not the same as {release.project.name!r}",
11261128
queue="error",
11271129
)
11281130
]
1129-
assert request.route_path.calls == [
1131+
assert db_request.route_path.calls == [
11301132
pretend.call(
11311133
"manage.project.release",
11321134
project_name=release.project.name,

‎tests/unit/packaging/test_models.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -260,8 +260,7 @@ def test_urls(self, db_session, home_page, download_url, project_urls, expected)
260260
for urlspec in project_urls:
261261
db_session.add(
262262
Dependency(
263-
name=release.project.name,
264-
version=release.version,
263+
release=release,
265264
kind=DependencyKind.project_url.value,
266265
specifier=urlspec,
267266
)

‎tests/unit/search/test_tasks.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ def test_project_docs(db_session):
5757
"latest_version": first(prs, key=lambda r: not r.is_prerelease).version,
5858
},
5959
}
60-
for p, prs in sorted(releases.items(), key=lambda x: x[0].name.lower())
60+
for p, prs in sorted(releases.items(), key=lambda x: x[0].id)
6161
]
6262

6363

‎tests/unit/utils/test_project.py

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ def test_remove_project(db_request, flash):
9595
user = UserFactory.create()
9696
project = ProjectFactory.create(name="foo")
9797
release = ReleaseFactory.create(project=project)
98-
FileFactory.create(name=project.name, version=release.version, filename="who cares")
98+
FileFactory.create(release=release, filename="who cares")
9999
RoleFactory.create(user=user, project=project)
100-
DependencyFactory.create(name=project.name, version=release.version)
100+
DependencyFactory.create(release=release)
101101

102102
db_request.user = user
103103
db_request.remote_addr = "192.168.1.1"
@@ -113,13 +113,20 @@ def test_remove_project(db_request, flash):
113113
assert db_request.session.flash.calls == []
114114

115115
assert not (db_request.db.query(Role).filter(Role.project == project).count())
116-
assert not (db_request.db.query(File).filter(File.name == project.name).count())
117116
assert not (
118-
db_request.db.query(Dependency).filter(Dependency.name == project.name).count()
117+
db_request.db.query(File)
118+
.join(Release)
119+
.join(Project)
120+
.filter(Release.project == project)
121+
.count()
119122
)
120123
assert not (
121-
db_request.db.query(Release).filter(Release.name == project.name).count()
124+
db_request.db.query(Dependency)
125+
.join(Release)
126+
.filter(Release.project == project)
127+
.count()
122128
)
129+
assert not (db_request.db.query(Release).filter(Release.project == project).count())
123130
assert not (
124131
db_request.db.query(Project).filter(Project.name == project.name).count()
125132
)

0 commit comments

Comments
 (0)