Skip to content

Commit

Permalink
Avoid caching page headers (mozilla#3125)
Browse files Browse the repository at this point in the history
* Cache /insights data, rather than entire /insights page
* Cache Contributors data, rather than entire Contributors views
  • Loading branch information
mathjazz authored Mar 4, 2024
1 parent b9b70cc commit e8f2b19
Show file tree
Hide file tree
Showing 10 changed files with 81 additions and 81 deletions.
29 changes: 7 additions & 22 deletions pontoon/contributors/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -223,42 +223,30 @@ def mock_users_translations_counts():


@pytest.mark.django_db
def test_default_period(
member, mock_contributors_render, mock_users_translations_counts
):
def test_default_period(member, mock_contributors_render):
"""
Calling the top contributors should result in period being None.
"""
member.client.get("/contributors/")
assert mock_contributors_render.call_args[0][0]["period"] is None
assert mock_users_translations_counts.call_args[0][0] is None


@pytest.mark.django_db
def test_invalid_period(member, mock_users_translations_counts):
def test_invalid_period(member, mock_contributors_render):
"""
Checks how view handles invalid period, it result in period being None - displays all data.
"""
# If period parameter is invalid value
with patch.object(
views.ContributorsView, "render_to_response", return_value=HttpResponse("")
) as mock_contributors_render:
member.client.get("/contributors/?period=invalidperiod")
assert mock_contributors_render.call_args[0][0]["period"] is None
assert mock_users_translations_counts.call_args[0][0] is None
member.client.get("/contributors/?period=invalidperiod")
assert mock_contributors_render.call_args[0][0]["period"] is None

# Period shouldn't be negative integer
# The ContributorsView URL is cached, so we need a new mock
with patch.object(
views.ContributorsView, "render_to_response", return_value=HttpResponse("")
) as mock_contributors_render:
member.client.get("/contributors/?period=-6")
assert mock_contributors_render.call_args[0][0]["period"] is None
assert mock_users_translations_counts.call_args[0][0] is None
member.client.get("/contributors/?period=-6")
assert mock_contributors_render.call_args[0][0]["period"] is None


@pytest.mark.django_db
def test_given_period(member, mock_contributors_render, mock_users_translations_counts):
def test_given_period(member, mock_contributors_render):
"""
Checks if view sets and returns data for right period.
"""
Expand All @@ -269,6 +257,3 @@ def test_given_period(member, mock_contributors_render, mock_users_translations_
):
member.client.get("/contributors/?period=6")
assert mock_contributors_render.call_args[0][0]["period"] == 6
assert mock_users_translations_counts.call_args[0][0] == aware_datetime(
2015, 1, 5
)
5 changes: 1 addition & 4 deletions pontoon/contributors/urls.py
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
from django.urls import path, register_converter
from django.urls.converters import StringConverter
from django.views.decorators.cache import cache_page
from django.views.generic import RedirectView

from pontoon.settings import VIEW_CACHE_TIMEOUT

from . import views


Expand All @@ -28,7 +25,7 @@ class UsernameConverter(StringConverter):
# List contributors
path(
"contributors/",
cache_page(VIEW_CACHE_TIMEOUT)(views.ContributorsView.as_view()),
views.ContributorsView.as_view(),
name="pontoon.contributors",
),
# Contributor profile by username
Expand Down
30 changes: 22 additions & 8 deletions pontoon/contributors/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from django.contrib import messages
from django.contrib.auth.decorators import login_required
from django.contrib.auth.models import User
from django.core.cache import cache
from django.core.exceptions import ValidationError
from django.db import transaction
from django.db.models import Q
Expand All @@ -24,6 +25,7 @@
from pontoon.base.models import Locale, Project, UserProfile
from pontoon.base.utils import require_AJAX, get_locale_or_redirect
from pontoon.contributors import utils
from pontoon.settings import VIEW_CACHE_TIMEOUT
from pontoon.uxactionlog.utils import log_ux_action


Expand Down Expand Up @@ -465,14 +467,26 @@ def get_context_data(self, **kwargs):
period = None
start_date = None

context["contributors"] = utils.users_with_translations_counts(
start_date,
self.contributors_filter(**kwargs)
& Q(user__isnull=False)
& Q(user__profile__system_user=False),
kwargs.get("locale"),
100,
)
qs = str(self.contributors_filter(**kwargs)).replace(" ", "")
key = f"{__name__}.{qs}.{period}"

# Cannot use cache.get_or_set(), because it always calls the slow function
# users_with_translations_count(). The reason we use cache in first place is to
# avoid that.
contributors = cache.get(key)
if not contributors:
contributors = utils.users_with_translations_counts(
start_date,
self.contributors_filter(**kwargs)
& Q(user__isnull=False)
& Q(user__profile__system_user=False),
kwargs.get("locale"),
100,
)
cache.set(key, contributors, VIEW_CACHE_TIMEOUT)

context["contributors"] = contributors

context["period"] = period
return context

Expand Down
5 changes: 1 addition & 4 deletions pontoon/insights/urls.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
from django.urls import path
from django.views.decorators.cache import cache_page

from pontoon.settings import VIEW_CACHE_TIMEOUT

from . import views

urlpatterns = [
# Insights page
path(
"insights/",
cache_page(VIEW_CACHE_TIMEOUT)(views.insights),
views.insights,
name="pontoon.insights",
),
]
31 changes: 25 additions & 6 deletions pontoon/insights/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

from dateutil.relativedelta import relativedelta
from django.conf import settings
from django.core.cache import cache
from django.core.exceptions import ImproperlyConfigured
from django.shortcuts import render
from django.utils import timezone
Expand All @@ -16,17 +17,35 @@ def insights(request):
if not settings.ENABLE_INSIGHTS:
raise ImproperlyConfigured("ENABLE_INSIGHTS variable not set in settings.")

# Cannot use cache.get_or_set(), because it always calls the slow function
# get_global_pretranslation_quality(). The reason we use cache in first place is to
# avoid that.

team_pt_key = f"/{__name__}/team_pretranslation_quality"
team_pretranslation_quality = cache.get(team_pt_key)
if not team_pretranslation_quality:
team_pretranslation_quality = get_global_pretranslation_quality(
"locale", "code"
)
cache.set(team_pt_key, team_pretranslation_quality, settings.VIEW_CACHE_TIMEOUT)

project_pt_key = f"/{__name__}/project_pretranslation_quality"
project_pretranslation_quality = cache.get(project_pt_key)
if not project_pretranslation_quality:
project_pretranslation_quality = get_global_pretranslation_quality(
"entity__resource__project", "slug"
)
cache.set(
project_pt_key, project_pretranslation_quality, settings.VIEW_CACHE_TIMEOUT
)

return render(
request,
"insights/insights.html",
{
"start_date": timezone.now() - relativedelta(years=1),
"end_date": timezone.now(),
"team_pretranslation_quality": get_global_pretranslation_quality(
"locale", "code"
),
"project_pretranslation_quality": get_global_pretranslation_quality(
"entity__resource__project", "slug"
),
"team_pretranslation_quality": team_pretranslation_quality,
"project_pretranslation_quality": project_pretranslation_quality,
},
)
4 changes: 1 addition & 3 deletions pontoon/localizations/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,7 @@
# Localization contributors
path(
"contributors/",
cache_page(VIEW_CACHE_TIMEOUT)(
views.LocalizationContributorsView.as_view()
),
views.LocalizationContributorsView.as_view(),
name="pontoon.localizations.ajax.contributors",
),
# Project insights
Expand Down
5 changes: 0 additions & 5 deletions pontoon/projects/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,11 +78,6 @@ def test_project_top_contributors(client, project_a, project_b):
project_a_contributor
]

# The ProjectContributorsView URL is cached, so we need a new mock
with patch(
"pontoon.projects.views.ProjectContributorsView.render_to_response",
return_value=HttpResponse(""),
) as mock_render:
client.get(
f"/projects/{project_b.slug}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
Expand Down
4 changes: 1 addition & 3 deletions pontoon/projects/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@
# Project contributors
path(
"contributors/",
cache_page(VIEW_CACHE_TIMEOUT)(
views.ProjectContributorsView.as_view()
),
views.ProjectContributorsView.as_view(),
name="pontoon.projects.ajax.contributors",
),
# Project insights
Expand Down
45 changes: 22 additions & 23 deletions pontoon/teams/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,29 +170,28 @@ def test_users_permissions_for_ajax_permissions_view(


@pytest.mark.django_db
def test_locale_top_contributors(client, translation_a, locale_b):
@patch(
"pontoon.teams.views.LocaleContributorsView.render_to_response",
return_value=HttpResponse(""),
)
def test_locale_top_contributors(mock_render, client, translation_a, locale_b):
"""
Tests if the view returns top contributors specific for given locale.
"""
with patch(
"pontoon.teams.views.LocaleContributorsView.render_to_response",
return_value=HttpResponse(""),
) as mock_render:
client.get(
f"/{translation_a.locale.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
assert mock_render.call_args[0][0]["locale"] == translation_a.locale
assert list(mock_render.call_args[0][0]["contributors"]) == [translation_a.user]

# The LocaleContributorsView URL is cached, so we need a new mock
with patch(
"pontoon.teams.views.LocaleContributorsView.render_to_response",
return_value=HttpResponse(""),
) as mock_render:
client.get(
f"/{locale_b.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)
assert mock_render.call_args[0][0]["locale"] == locale_b
assert list(mock_render.call_args[0][0]["contributors"]) == []
client.get(
f"/{translation_a.locale.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)

response_context = mock_render.call_args[0][0]
assert response_context["locale"] == translation_a.locale
assert list(response_context["contributors"]) == [translation_a.user]

client.get(
f"/{locale_b.code}/ajax/contributors/",
HTTP_X_REQUESTED_WITH="XMLHttpRequest",
)

response_context = mock_render.call_args[0][0]
assert response_context["locale"] == locale_b
assert list(response_context["contributors"]) == []
4 changes: 1 addition & 3 deletions pontoon/teams/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,7 @@
# Team contributors
path(
"contributors/",
cache_page(VIEW_CACHE_TIMEOUT)(
views.LocaleContributorsView.as_view()
),
views.LocaleContributorsView.as_view(),
name="pontoon.teams.ajax.contributors",
),
# Team insights
Expand Down

0 comments on commit e8f2b19

Please sign in to comment.