Skip to content

Commit

Permalink
Redirects: limit to 100 per project (#11028)
Browse files Browse the repository at this point in the history
  • Loading branch information
stsewd authored Jan 15, 2024
1 parent 1703827 commit 13ab601
Show file tree
Hide file tree
Showing 9 changed files with 101 additions and 19 deletions.
2 changes: 0 additions & 2 deletions docs/user/api/v3.rst
Original file line number Diff line number Diff line change
Expand Up @@ -1244,8 +1244,6 @@ Redirect create
* ``page`` and ``exact`` types require ``from_url`` and ``to_url``.
* ``clean_url_to_html`` and ``html_to_clean_url`` types do not require ``from_url`` and ``to_url``.

- Forced redirects are not enabled by default,
you can ask for them to be enabled via support.
- Position starts at 0 and is used to order redirects.

**Example response**:
Expand Down
6 changes: 3 additions & 3 deletions docs/user/user-defined-redirects.rst
Original file line number Diff line number Diff line change
Expand Up @@ -126,10 +126,10 @@ users will be redirected to the new URL.
Limitations and observations
~~~~~~~~~~~~~~~~~~~~~~~~~~~~

- |org_brand| users are limited to 100 redirects per project,
and |com_brand| users have a number of redirects limited by their plan.
- By default, redirects only apply on pages that don't exist.
**Forced redirects** allow you to apply redirects on existing pages,
but incur a small performance penalty, so aren't enabled by default.
You can ask for them to be enabled via support.
**Forced redirects** allow you to apply redirects on existing pages.
- Redirects aren't applied on :doc:`previews of pull requests </pull-requests>`.
You should treat these domains as ephemeral and not rely on them for user-facing content.
- You can redirect to URLs outside Read the Docs,
Expand Down
2 changes: 0 additions & 2 deletions readthedocs/api/v3/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -944,8 +944,6 @@ class Meta:
"position",
"_links",
]
# TODO: allow editing this field for projects that have this feature enabled.
read_only_fields = ["force"]

def __init__(self, *args, **kwargs):
super().__init__(*args, **kwargs)
Expand Down
11 changes: 4 additions & 7 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -785,13 +785,10 @@ def __init__(self, *args, **kwargs):
self.fields["enabled"].widget = forms.CheckboxInput()
self.fields["enabled"].empty_value = False

if self.project.has_feature(Feature.ALLOW_FORCED_REDIRECTS):
# Remove the nullable option from the form.
# TODO: remove after migration.
self.fields["force"].widget = forms.CheckboxInput()
self.fields["force"].empty_value = False
else:
self.fields.pop("force")
# Remove the nullable option from the form.
# TODO: remove after migration.
self.fields["force"].widget = forms.CheckboxInput()
self.fields["force"].empty_value = False

def clean_project(self):
return self.project
Expand Down
5 changes: 0 additions & 5 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1951,7 +1951,6 @@ def add_features(sender, **kwargs):
CONDA_APPEND_CORE_REQUIREMENTS = "conda_append_core_requirements"
ALL_VERSIONS_IN_HTML_CONTEXT = "all_versions_in_html_context"
RECORD_404_PAGE_VIEWS = "record_404_page_views"
ALLOW_FORCED_REDIRECTS = "allow_forced_redirects"
DISABLE_PAGEVIEWS = "disable_pageviews"
RESOLVE_PROJECT_FROM_HEADER = "resolve_project_from_header"
USE_PROXIED_APIS_WITH_PREFIX = "use_proxied_apis_with_prefix"
Expand Down Expand Up @@ -2020,10 +2019,6 @@ def add_features(sender, **kwargs):
RECORD_404_PAGE_VIEWS,
_("Proxito: Record 404s page views."),
),
(
ALLOW_FORCED_REDIRECTS,
_("Proxito: Allow forced redirects."),
),
(
DISABLE_PAGEVIEWS,
_("Proxito: Disable all page views"),
Expand Down
62 changes: 62 additions & 0 deletions readthedocs/redirects/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
PAGE_REDIRECT,
)
from readthedocs.redirects.models import Redirect
from readthedocs.subscriptions.constants import TYPE_REDIRECTS_LIMIT
from readthedocs.subscriptions.products import RTDProductFeature


@override_settings(RTD_ALLOW_ORGANIZATIONS=False)
Expand Down Expand Up @@ -224,6 +226,66 @@ def test_redirects_validations(self):
"Only one redirect of type `html_to_clean_url` is allowed per project.",
)

@override_settings(
RTD_DEFAULT_FEATURES=dict(
(
RTDProductFeature(
type=TYPE_REDIRECTS_LIMIT,
value=2,
).to_item(),
)
)
)
def test_redirects_limit(self):
self.assertEqual(self.project.redirects.all().count(), 1)
url = reverse("projects_redirects_create", args=[self.project.slug])
resp = self.client.post(
url,
data={
"redirect_type": EXACT_REDIRECT,
"from_url": "a",
"to_url": "b",
"http_status": 302,
},
)
self.assertEqual(resp.status_code, 302)

resp = self.client.post(
url,
data={
"redirect_type": EXACT_REDIRECT,
"from_url": "c",
"to_url": "d",
"http_status": 302,
},
)
self.assertEqual(resp.status_code, 200)
self.assertContains(
resp,
"This project has reached the limit of 2 redirects.",
)
self.assertEqual(self.project.redirects.all().count(), 2)

# Update works
resp = self.client.post(
reverse(
"projects_redirects_edit", args=[self.project.slug, self.redirect.pk]
),
data={
"redirect_type": PAGE_REDIRECT,
"http_status": 302,
},
)
self.assertEqual(resp.status_code, 302)

# Delete works
resp = self.client.post(
reverse(
"projects_redirects_delete", args=[self.project.slug, self.redirect.pk]
),
)
self.assertEqual(resp.status_code, 302)


@override_settings(RTD_ALLOW_ORGANIZATIONS=True)
class TestViewsWithOrganizations(TestViews):
Expand Down
28 changes: 28 additions & 0 deletions readthedocs/redirects/validators.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
from django.conf import settings
from django.core.exceptions import ValidationError
from django.utils.translation import gettext_lazy as _

from readthedocs.redirects.constants import (
CLEAN_URL_TO_HTML_REDIRECT,
EXACT_REDIRECT,
HTML_TO_CLEAN_URL_REDIRECT,
PAGE_REDIRECT,
)
from readthedocs.subscriptions.constants import TYPE_REDIRECTS_LIMIT
from readthedocs.subscriptions.products import get_feature


def validate_redirect(
Expand All @@ -18,6 +22,10 @@ def validate_redirect(
(used in forms), and in the Django Rest Framework serializer (used in the API).
Since DRF doesn't call the clean method of the model.
"""
# Check for the limit if we are creating a new redirect.
if not pk:
_check_redirects_limit(project, error_class)

if redirect_type in [EXACT_REDIRECT, PAGE_REDIRECT]:
if from_url.endswith("$rest"):
raise error_class("The $rest wildcard has been removed in favor of *.")
Expand All @@ -38,3 +46,23 @@ def validate_redirect(
raise error_class(
f"Only one redirect of type `{redirect_type}` is allowed per project."
)


def _check_redirects_limit(project, error_class):
"""Check if the project has reached the limit on the number of redirects."""
feature = get_feature(project, TYPE_REDIRECTS_LIMIT)
if feature.unlimited:
return

if project.redirects.count() >= feature.value:
msg = _(
f"This project has reached the limit of {feature.value} redirects."
" Consider replacing some of your redirects with a wildcard redirect."
)
if settings.ALLOW_PRIVATE_REPOS:
msg = _(
f"This project has reached the limit of {feature.value} redirects."
" Consider replacing some of your redirects with a wildcard redirect,"
" or upgrade your plan."
)
raise error_class(msg)
2 changes: 2 additions & 0 deletions readthedocs/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,8 @@ def RTD_DEFAULT_FEATURES(self):
RTDProductFeature(type=constants.TYPE_AUDIT_LOGS, value=self.RTD_AUDITLOGS_DEFAULT_RETENTION_DAYS).to_item(),
# Max number of concurrent builds.
RTDProductFeature(type=constants.TYPE_CONCURRENT_BUILDS, value=self.RTD_MAX_CONCURRENT_BUILDS).to_item(),
# Max number of redirects allowed per project.
RTDProductFeature(type=constants.TYPE_REDIRECTS_LIMIT, value=100).to_item(),
))

# A dictionary of Stripe products mapped to a RTDProduct object.
Expand Down
2 changes: 2 additions & 0 deletions readthedocs/subscriptions/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
TYPE_CUSTOM_URL = "urls"
TYPE_AUDIT_LOGS = "audit-logs"
TYPE_AUDIT_PAGEVIEWS = "audit-pageviews"
TYPE_REDIRECTS_LIMIT = "redirects-limit"

FEATURE_TYPES = (
(TYPE_CNAME, _("Custom domain")),
Expand All @@ -33,4 +34,5 @@
(TYPE_CUSTOM_URL, _("Custom URLs")),
(TYPE_AUDIT_LOGS, _("Audit logs")),
(TYPE_AUDIT_PAGEVIEWS, _("Audit logs for every page view")),
(TYPE_REDIRECTS_LIMIT, _("Redirects limit")),
)

0 comments on commit 13ab601

Please sign in to comment.