Skip to content

Render description HTML on upload #5818

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
May 12, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions tests/common/db/packaging.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@
BlacklistedProject,
Dependency,
DependencyKind,
Description,
File,
JournalEntry,
Project,
Release,
Role,
)
from warehouse.utils import readme

from .accounts import UserFactory
from .base import WarehouseFactory
Expand All @@ -41,6 +43,16 @@ class Meta:
name = factory.fuzzy.FuzzyText(length=12)


class DescriptionFactory(WarehouseFactory):
class Meta:
model = Description

id = factory.LazyFunction(uuid.uuid4)
raw = factory.fuzzy.FuzzyText(length=100)
html = factory.LazyAttribute(lambda o: readme.render(o.raw))
rendered_by = factory.LazyAttribute(lambda o: readme.renderer_version())


class ReleaseFactory(WarehouseFactory):
class Meta:
model = Release
Expand All @@ -54,6 +66,7 @@ class Meta:
_pypi_ordering = factory.Sequence(lambda n: n)

uploader = factory.SubFactory(UserFactory)
description = factory.SubFactory(DescriptionFactory)


class FileFactory(WarehouseFactory):
Expand Down
11 changes: 7 additions & 4 deletions tests/unit/legacy/api/test_json.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from ....common.db.accounts import UserFactory
from ....common.db.packaging import (
DescriptionFactory,
FileFactory,
JournalEntryFactory,
ProjectFactory,
Expand Down Expand Up @@ -187,7 +188,9 @@ def test_detail_renders(self, pyramid_config, db_request, db_session):
ReleaseFactory.create(
project=project,
version="3.0",
description_content_type=description_content_type,
description=DescriptionFactory.create(
content_type=description_content_type
),
)
]

Expand Down Expand Up @@ -239,7 +242,7 @@ def test_detail_renders(self, pyramid_config, db_request, db_session):
"bugtrack_url": None,
"classifiers": [],
"description_content_type": description_content_type,
"description": None,
"description": releases[-1].description.raw,
"docs_url": "/the/fake/url/",
"download_url": None,
"downloads": {"last_day": -1, "last_week": -1, "last_month": -1},
Expand Down Expand Up @@ -384,8 +387,8 @@ def test_minimal_renders(self, pyramid_config, db_request):
"author_email": None,
"bugtrack_url": None,
"classifiers": [],
"description_content_type": None,
"description": None,
"description_content_type": release.description.content_type,
"description": release.description.raw,
"docs_url": None,
"download_url": None,
"downloads": {"last_day": -1, "last_week": -1, "last_month": -1},
Expand Down
2 changes: 1 addition & 1 deletion tests/unit/legacy/api/xmlrpc/test_xmlrpc.py
Original file line number Diff line number Diff line change
Expand Up @@ -605,7 +605,7 @@ def test_release_data(db_request):
"maintainer": release.maintainer,
"maintainer_email": release.maintainer_email,
"summary": release.summary,
"description": release.description,
"description": release.description.raw,
"license": release.license,
"keywords": release.keywords,
"platform": release.platform,
Expand Down
9 changes: 7 additions & 2 deletions tests/unit/packaging/test_init.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
from warehouse.accounts.models import Email, User
from warehouse.packaging.interfaces import IDocsStorage, IFileStorage
from warehouse.packaging.models import File, Project, Release, Role
from warehouse.packaging.tasks import compute_trending
from warehouse.packaging.tasks import compute_trending, update_description_html


@pytest.mark.parametrize("with_trending", [True, False])
Expand Down Expand Up @@ -106,5 +106,10 @@ def key_factory(keystring, iterate_on=None):

if with_trending:
assert config.add_periodic_task.calls == [
pretend.call(crontab(minute=0, hour=3), compute_trending)
pretend.call(crontab(minute="*/5"), update_description_html),
pretend.call(crontab(minute=0, hour=3), compute_trending),
]
else:
assert config.add_periodic_task.calls == [
pretend.call(crontab(minute="*/5"), update_description_html)
]
32 changes: 29 additions & 3 deletions tests/unit/packaging/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
from google.cloud.bigquery import Row

from warehouse.cache.origin import IOriginCache
from warehouse.packaging.models import Project
from warehouse.packaging.tasks import compute_trending
from warehouse.packaging.models import Description, Project
from warehouse.packaging.tasks import compute_trending, update_description_html
from warehouse.utils import readme

from ...common.db.packaging import ProjectFactory
from ...common.db.packaging import DescriptionFactory, ProjectFactory


class TestComputeTrending:
Expand Down Expand Up @@ -108,3 +109,28 @@ def find_service(iface=None, name=None):
projects[1].name: 2,
projects[2].name: -1,
}


def test_update_description_html(monkeypatch, db_request):
current_version = "24.0"
previous_version = "23.0"

monkeypatch.setattr(readme, "renderer_version", lambda: current_version)

descriptions = [
DescriptionFactory.create(html="rendered", rendered_by=current_version),
DescriptionFactory.create(html="not this one", rendered_by=previous_version),
DescriptionFactory.create(html="", rendered_by=""), # Initial migration state
]

update_description_html(db_request)

assert set(
db_request.db.query(
Description.raw, Description.html, Description.rendered_by
).all()
) == {
(descriptions[0].raw, "rendered", current_version),
(descriptions[1].raw, readme.render(descriptions[1].raw), current_version),
(descriptions[2].raw, readme.render(descriptions[2].raw), current_version),
}
52 changes: 50 additions & 2 deletions tests/unit/packaging/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
from ...common.db.accounts import UserFactory
from ...common.db.classifiers import ClassifierFactory
from ...common.db.packaging import (
DescriptionFactory,
FileFactory,
ProjectFactory,
ReleaseFactory,
Expand Down Expand Up @@ -145,15 +146,62 @@ def test_normalizing_version_redirects(self, db_request):
pretend.call(name=release.project.name, version=release.version)
]

def test_detail_rendered(self, db_request):
users = [UserFactory.create(), UserFactory.create(), UserFactory.create()]
project = ProjectFactory.create()
releases = [
ReleaseFactory.create(
project=project,
version=v,
description=DescriptionFactory.create(
raw="unrendered description",
html="rendered description",
content_type="text/plain",
),
)
for v in ["1.0", "2.0", "3.0", "4.0.dev0"]
]
files = [
FileFactory.create(
release=r,
filename="{}-{}.tar.gz".format(project.name, r.version),
python_version="source",
)
for r in releases
]

# Create a role for each user
for user in users:
RoleFactory.create(user=user, project=project)

# Add an extra role for one user, to ensure deduplication
RoleFactory.create(user=users[0], project=project, role_name="another role")

result = views.release_detail(releases[1], db_request)

assert result == {
"project": project,
"release": releases[1],
"files": [files[1]],
"description": "rendered description",
"latest_version": project.latest_version,
"all_versions": [
(r.version, r.created, r.is_prerelease) for r in reversed(releases)
],
"maintainers": sorted(users, key=lambda u: u.username.lower()),
"license": None,
}

def test_detail_renders(self, monkeypatch, db_request):
users = [UserFactory.create(), UserFactory.create(), UserFactory.create()]
project = ProjectFactory.create()
releases = [
ReleaseFactory.create(
project=project,
version=v,
description="unrendered description",
description_content_type="text/plain",
description=DescriptionFactory.create(
raw="unrendered description", html="", content_type="text/plain"
),
)
for v in ["1.0", "2.0", "3.0", "4.0.dev0"]
]
Expand Down
6 changes: 6 additions & 0 deletions tests/unit/search/test_tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ def test_project_docs(db_session):
"normalized_name": p.normalized_name,
"version": [r.version for r in prs],
"latest_version": first(prs, key=lambda r: not r.is_prerelease).version,
"description": first(
prs, key=lambda r: not r.is_prerelease
).description.raw,
},
}
for p, prs in sorted(releases.items(), key=lambda x: x[0].id)
Expand Down Expand Up @@ -82,6 +85,9 @@ def test_single_project_doc(db_session):
"normalized_name": p.normalized_name,
"version": [r.version for r in prs],
"latest_version": first(prs, key=lambda r: not r.is_prerelease).version,
"description": first(
prs, key=lambda r: not r.is_prerelease
).description.raw,
},
}
for p, prs in sorted(releases.items(), key=lambda x: x[0].name.lower())
Expand Down
72 changes: 40 additions & 32 deletions warehouse/forklift/legacy.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,15 @@
BlacklistedProject,
Dependency,
DependencyKind,
Description,
File,
Filename,
JournalEntry,
Project,
Release,
Role,
)
from warehouse.utils import http
from warehouse.utils import http, readme

MAX_FILESIZE = 60 * 1024 * 1024 # 60M
MAX_SIGSIZE = 8 * 1024 # 8K
Expand Down Expand Up @@ -928,35 +929,38 @@ def file_upload(request):
if project.name != form.name.data:
project.name = form.name.data

# Uploading should prevent broken rendered descriptions.
# Temporarily disabled, see
# https://github.com/pypa/warehouse/issues/4079
# if form.description.data:
# description_content_type = form.description_content_type.data
# if not description_content_type:
# description_content_type = "text/x-rst"
# rendered = readme.render(
# form.description.data, description_content_type, use_fallback=False
# )

# if rendered is None:
# if form.description_content_type.data:
# message = (
# "The description failed to render "
# "for '{description_content_type}'."
# ).format(description_content_type=description_content_type)
# else:
# message = (
# "The description failed to render "
# "in the default format of reStructuredText."
# )
# raise _exc_with_message(
# HTTPBadRequest,
# "{message} See {projecthelp} for more information.".format(
# message=message,
# projecthelp=request.help_url(_anchor="description-content-type"),
# ),
# ) from None
# Render our description so we can save from having to render this data every time
# we load a project description page.
rendered = None
if form.description.data:
description_content_type = form.description_content_type.data
# if not description_content_type:
# description_content_type = "text/x-rst"
rendered = readme.render(
form.description.data, description_content_type # use_fallback=False
)

# Uploading should prevent broken rendered descriptions.
# Temporarily disabled, see
# https://github.com/pypa/warehouse/issues/4079
# if rendered is None:
# if form.description_content_type.data:
# message = (
# "The description failed to render "
# "for '{description_content_type}'."
# ).format(description_content_type=description_content_type)
# else:
# message = (
# "The description failed to render "
# "in the default format of reStructuredText."
# )
# raise _exc_with_message(
# HTTPBadRequest,
# "{message} See {projecthelp} for more information.".format(
# message=message,
# projecthelp=request.help_url(_anchor="description-content-type"),
# ),
# ) from None

try:
canonical_version = packaging.utils.canonicalize_version(form.version.data)
Expand Down Expand Up @@ -1001,15 +1005,19 @@ def file_upload(request):
)
),
canonical_version=canonical_version,
description=Description(
content_type=form.description_content_type.data,
raw=form.description.data or "",
html=rendered or "",
rendered_by=readme.renderer_version(),
),
**{
k: getattr(form, k).data
for k in {
# This is a list of all the fields in the form that we
# should pull off and insert into our new release.
"version",
"summary",
"description",
"description_content_type",
"license",
"author",
"author_email",
Expand Down
4 changes: 2 additions & 2 deletions warehouse/legacy/api/json.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ def json_release(release, request):
"name": project.name,
"version": release.version,
"summary": release.summary,
"description_content_type": release.description_content_type,
"description": release.description,
"description_content_type": release.description.content_type,
"description": release.description.raw,
"keywords": release.keywords,
"license": release.license,
"classifiers": list(release.classifiers),
Expand Down
4 changes: 2 additions & 2 deletions warehouse/legacy/api/xmlrpc/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -359,7 +359,7 @@ def release_data(request, package_name: str, version: str):
try:
release = (
request.db.query(Release)
.options(orm.undefer("description"))
.options(orm.joinedload(Release.description))
.join(Project)
.filter(
(Project.normalized_name == func.normalize_pep426_name(package_name))
Expand Down Expand Up @@ -390,7 +390,7 @@ def release_data(request, package_name: str, version: str):
"maintainer": _clean_for_xml(release.maintainer),
"maintainer_email": _clean_for_xml(release.maintainer_email),
"summary": _clean_for_xml(release.summary),
"description": _clean_for_xml(release.description),
"description": _clean_for_xml(release.description.raw),
"license": _clean_for_xml(release.license),
"keywords": _clean_for_xml(release.keywords),
"platform": release.platform,
Expand Down
Loading