Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add limit and max_age params to RSS feeds #7117

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Add limit and max_age params to RSS feeds #7117

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
116 changes: 116 additions & 0 deletions tests/unit/rss/test_views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,12 @@
import datetime

import pretend
import pytest

from pyramid.httpexceptions import HTTPBadRequest

from warehouse.rss import views as rss
from warehouse.utils import now

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

Expand Down Expand Up @@ -44,6 +48,72 @@ def test_rss_updates(db_request):
assert db_request.response.content_type == "text/xml"


def test_rss_updates_limit(db_request):
db_request.params = {"limit": 2}

db_request.find_service = pretend.call_recorder(
lambda *args, **kwargs: pretend.stub(
enabled=False, csp_policy=pretend.stub(), merge=lambda _: None
)
)

db_request.session = pretend.stub()

project1 = ProjectFactory.create()
project2 = ProjectFactory.create()

release1 = ReleaseFactory.create(project=project1)
release1.created = datetime.date(2011, 1, 1)
yeraydiazdiaz marked this conversation as resolved.
Show resolved Hide resolved
release2 = ReleaseFactory.create(project=project2)
release2.created = datetime.date(2012, 1, 1)
release3 = ReleaseFactory.create(project=project1)
release3.created = datetime.date(2013, 1, 1)

assert rss.rss_updates(db_request) == {"latest_releases": [release3, release2]}
assert db_request.response.content_type == "text/xml"


def test_rss_updates_max_age(db_request):
db_request.params = {"max_age": 150}

db_request.find_service = pretend.call_recorder(
lambda *args, **kwargs: pretend.stub(
enabled=False, csp_policy=pretend.stub(), merge=lambda _: None
)
)

db_request.session = pretend.stub()

project1 = ProjectFactory.create()

release1 = ReleaseFactory.create(project=project1)
release1.created = now() - datetime.timedelta(seconds=100)
release2 = ReleaseFactory.create(project=project1)
release2.created = now() - datetime.timedelta(seconds=200)

assert rss.rss_updates(db_request) == {"latest_releases": [release1]}
assert db_request.response.content_type == "text/xml"


def test_rss_updates_max_age_invalid(db_request):
db_request.params = {"max_age": "foo"}

db_request.find_service = pretend.call_recorder(
lambda *args, **kwargs: pretend.stub(
enabled=False, csp_policy=pretend.stub(), merge=lambda _: None
)
)

db_request.session = pretend.stub()

with pytest.raises(HTTPBadRequest) as excinfo:
rss.rss_updates(db_request)

resp = excinfo.value

assert resp.status_code == 400


def test_rss_packages(db_request):
db_request.find_service = pretend.call_recorder(
lambda *args, **kwargs: pretend.stub(
Expand All @@ -66,3 +136,49 @@ def test_rss_packages(db_request):

assert rss.rss_packages(db_request) == {"newest_projects": [project3, project1]}
assert db_request.response.content_type == "text/xml"


def test_rss_packages_limit(db_request):
db_request.params = {"limit": 1}

db_request.find_service = pretend.call_recorder(
lambda *args, **kwargs: pretend.stub(
enabled=False, csp_policy=pretend.stub(), merge=lambda _: None
)
)

db_request.session = pretend.stub()

project1 = ProjectFactory.create()
project1.created = datetime.date(2011, 1, 1)
ReleaseFactory.create(project=project1)

project2 = ProjectFactory.create()
project2.created = datetime.date(2012, 1, 1)
ReleaseFactory.create(project=project2)

assert rss.rss_packages(db_request) == {"newest_projects": [project2]}
assert db_request.response.content_type == "text/xml"


def test_rss_packages_max_age(db_request):
db_request.params = {"max_age": 150}

db_request.find_service = pretend.call_recorder(
lambda *args, **kwargs: pretend.stub(
enabled=False, csp_policy=pretend.stub(), merge=lambda _: None
)
)

db_request.session = pretend.stub()

project1 = ProjectFactory.create()
project1.created = now() - datetime.timedelta(seconds=100)
ReleaseFactory.create(project=project1)

project2 = ProjectFactory.create()
project2.created = now() - datetime.timedelta(seconds=200)
ReleaseFactory.create(project=project2)

assert rss.rss_packages(db_request) == {"newest_projects": [project1]}
assert db_request.response.content_type == "text/xml"
42 changes: 36 additions & 6 deletions warehouse/rss/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,31 @@
# See the License for the specific language governing permissions and
# limitations under the License.

from datetime import timedelta

from pyramid.httpexceptions import HTTPBadRequest
from pyramid.view import view_config
from sqlalchemy.orm import joinedload

from warehouse.cache.origin import origin_cache
from warehouse.packaging.models import Project, Release
from warehouse.utils import now
from warehouse.xml import XML_CSP

DEFAULT_RESULTS = 40
MAX_RESULTS = 200


def _get_int_query_param(request, param, default=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: I feel defaulting to None might be dangerous here as the caller might expect an integer. The two options I see is raising an expection or returning a sentinel value (maybe -1 or 0 would make sense?).

value = request.params.get(param)
if not value:
# Return default if 'param' is absent or has an empty value.
return default
try:
return int(value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it'd be worth asserting it must be a positive integer as well?

except ValueError:
raise HTTPBadRequest(f"'{param}' must be an integer.") from None


@view_config(
route_name="rss.updates",
Expand All @@ -39,11 +57,17 @@ def rss_updates(request):
request.db.query(Release)
.options(joinedload(Release.project))
.order_by(Release.created.desc())
.limit(40)
.all()
)

return {"latest_releases": latest_releases}
max_age = _get_int_query_param(request, "max_age")
if max_age is not None:
created_since = now() - timedelta(seconds=max_age)
latest_releases = latest_releases.filter(Release.created > created_since)

limit = min(_get_int_query_param(request, "limit", DEFAULT_RESULTS), MAX_RESULTS)
latest_releases = latest_releases.limit(limit)

return {"latest_releases": latest_releases.all()}


@view_config(
Expand All @@ -67,8 +91,14 @@ def rss_packages(request):
request.db.query(Project)
.options(joinedload(Project.releases, innerjoin=True))
.order_by(Project.created.desc())
.limit(40)
.all()
)

return {"newest_projects": newest_projects}
max_age = _get_int_query_param(request, "max_age")
if max_age is not None:
created_since = now() - timedelta(seconds=max_age)
newest_projects = newest_projects.filter(Project.created > created_since)

limit = min(_get_int_query_param(request, "limit", DEFAULT_RESULTS), MAX_RESULTS)
newest_projects = newest_projects.limit(limit)

return {"newest_projects": newest_projects.all()}