Skip to content

Commit bf74e76

Browse files
committed
Cache 404 pages per-version
Caches are our most expensive requests since at this point we check for multiple dirhtml files in the storage, for redirects, for a custom 404 page and we finally render the default Read the Docs' 404 page (Maze). This new code checks for these scenarios (dirhtml, redirect, custom, default) and return the cached response if it exists before performing any other operation. The default 404 page response is cached globally since it's the same for all the projects. Although, before returning it, we check that was the response we served the first time and not the custom 404 one.
1 parent 17bcdb4 commit bf74e76

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

readthedocs/proxito/views/serve.py

Lines changed: 62 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
from urllib.parse import urlparse
77

88
from django.conf import settings
9+
from django.core.cache import cache
910
from django.core.files.storage import get_storage_class
1011
from django.http import Http404, HttpResponse, HttpResponseRedirect
1112
from django.shortcuts import render
@@ -187,6 +188,59 @@ def get(self, request, proxito_path, template_name='404.html'):
187188
version_slug=version_slug,
188189
filename=kwargs.get('filename', ''),
189190
)
191+
# Cache checks if cached,
192+
# 1. dirhtml response for this path
193+
# 2. redirect response for this path
194+
# 3. custom 404 response for this version
195+
# 4. default 404 response for this version
196+
197+
dirhtml_response = cache.get(f'dirhtml_404+{proxito_path}')
198+
if dirhtml_response:
199+
log.info(
200+
'Returning cached dirhtml response. project=%s version=%s',
201+
final_project.slug,
202+
version_slug,
203+
)
204+
return dirhtml_response
205+
206+
redirect_response = cache.get(f'user_redirect_on_404+{proxito_path}')
207+
if redirect_response:
208+
log.info(
209+
'Returning cached redirect response. project=%s version=%s',
210+
final_project.slug,
211+
version_slug,
212+
)
213+
return redirect_response
214+
215+
# The only way to get an improvement here is to use the ``project.slug``
216+
# and ``version.slug`` as ``cache_key``; that way all the 404s pages for
217+
# a specific version will return the same page. Using default
218+
# ``cache_page`` won't help on a scanning, since the URL will be
219+
# different on each request. We can do this because the 404 page is
220+
# exactly the same per-version.
221+
custom_404_response = cache.get(f'custom_404_{final_project.slug}+{version_slug}')
222+
if custom_404_response:
223+
log.info(
224+
'Returning cached custom 404 page. project=%s version=%s',
225+
final_project.slug,
226+
version_slug,
227+
)
228+
return custom_404_response
229+
230+
if cache.get(f'default_404_{final_project.slug}+{version_slug}'):
231+
default_404_response = cache.get(f'default_404_page')
232+
if default_404_response:
233+
log.info(
234+
'Returning cached default 404 page. project=%s version=%s',
235+
final_project.slug,
236+
version_slug,
237+
)
238+
return default_404_response
239+
log.debug(
240+
'Cached 404 responses missed. project=%s version=%s',
241+
final_project.slug,
242+
version_slug,
243+
)
190244

191245
storage_root_path = final_project.get_storage_path(
192246
type_='html',
@@ -225,7 +279,9 @@ def get(self, request, proxito_path, template_name='404.html'):
225279

226280
# TODO: decide if we need to check for infinite redirect here
227281
# (from URL == to URL)
228-
return HttpResponseRedirect(redirect_url)
282+
response = HttpResponseRedirect(redirect_url)
283+
cache.set('dirhtml_404+{proxito_path}', response, 60)
284+
return response
229285

230286
# ``redirect_filename`` is the path without ``/<lang>/<version>`` and
231287
# without query, starting with a ``/``. This matches our old logic:
@@ -253,7 +309,9 @@ def get(self, request, proxito_path, template_name='404.html'):
253309
)
254310
if redirect_path and http_status:
255311
try:
256-
return self.get_redirect_response(request, redirect_path, proxito_path, http_status)
312+
redirect_response = self.get_redirect_response(request, redirect_path, proxito_path, http_status)
313+
cache.set(f'user_redirect_on_404+{proxito_path}', redirect_response, 60)
314+
return redirect_response
257315
except InfiniteRedirectException:
258316
# Continue with our normal 404 handling in this case
259317
pass
@@ -278,8 +336,10 @@ def get(self, request, proxito_path, template_name='404.html'):
278336
)
279337
resp = HttpResponse(storage.open(storage_filename_path).read())
280338
resp.status_code = 404
339+
cache.set(f'custom_404_{final_project.slug}+{version_slug}', resp, 60)
281340
return resp
282341

342+
cache.set(f'default_404_{final_project.slug}+{version_slug}', True, 60)
283343
raise Http404('No custom 404 page found.')
284344

285345

readthedocs/proxito/views/utils.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import os
22
import logging
33

4+
from django.core.cache import cache
45
from django.http import HttpResponse
56
from django.shortcuts import get_object_or_404, render
67

@@ -34,6 +35,7 @@ def proxito_404_page_handler(request, exception=None, template_name='404.html'):
3435

3536
resp = render(request, template_name)
3637
resp.status_code = 404
38+
cache.set('default_404_page', resp, 60)
3739
return resp
3840

3941

0 commit comments

Comments
 (0)