Resolver: don't use one global instance and implement caching (#10872)

* Addons: pre-fetch the custom `Domain` instance to resolve URLs

The `resolver.resolve` method that resolves the URL **always** check for the
canonical `Domain` of the canonical `Project`. This means that resolving
the URLs for multiple `Version` objects results in querying the exact query over
and over again.

Instead of repeating this query, we are passing the `Domain` object to
`resolver.resolve` method to avoid hiting the DB again since we already known
what's the canonical domain for that project, in case it has one.

I found this issue while tracking down why there were around ~150 queries to the
`projects_domain` table when hitting `/_/addons/` API endpoint on New Relic.

* Add tests for number of queries

* Add more test cases for Addons API endpoint

* Test for custom Domain on flyout versions

* Check `use_custom_domain` after knowing the `custom_domain`

* Use `functools.lru_cache` for project's domain resolution

Caching the project's domain resolution reduces the amount of queries required
considerably. It helps to render the addons response among others since it needs
to resolve the domain only once no matter how many URLs/versions are needed to
be resolved.

* Adapt old tests to clean the resolver's cache between each test

* Clear cache on tests

* Update more tests to match the Resolver cache

* Clear cache on footer tests

* Use a new instance of `Resolver()` each time it's needed

Follows Santos' idea from
#10878 (comment)

* Minor fix

* Revert comment

* Typo import

* Typo import

* Remove unused method

readthedocs/analytics/models.py

@@ -9,7 +9,7 @@
 from django.utils.translation import gettext_lazy as _
 
 from readthedocs.builds.models import Version
-from readthedocs.core.resolver import resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.projects.models import Feature, Project
 
 
@@ -131,6 +131,7 @@ def top_viewed_pages(
         )
 
         PageViewResult = namedtuple("PageViewResult", "path, url, count")
+        resolver = Resolver()
         result = []
         parsed_domain = urlparse(resolver.get_domain(project))
         default_version = project.get_default_version()

readthedocs/api/v2/serializers.py

@@ -6,7 +6,7 @@
 
 from readthedocs.api.v2.utils import normalize_build_command
 from readthedocs.builds.models import Build, BuildCommandResult, Version
-from readthedocs.core.resolver import resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
 from readthedocs.projects.models import Domain, Project
 
@@ -180,7 +180,7 @@ def get_canonical_url(self, obj):
         # relationships already cached from calling
         # get_docs_url early when serializing the project.
         project = self._get_project_serialized(obj).instance
-        return resolver.resolve_version(
+        return Resolver().resolve_version(
             project=project,
             version=obj,
         )

readthedocs/api/v3/serializers.py

@@ -12,7 +12,7 @@
 from taggit.serializers import TaggitSerializer, TagListSerializerField
 
 from readthedocs.builds.models import Build, Version
-from readthedocs.core.resolver import resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.core.utils import slugify
 from readthedocs.core.utils.extend import SettingsOverrideObject
 from readthedocs.oauth.models import RemoteOrganization, RemoteRepository
@@ -244,7 +244,7 @@ class VersionURLsSerializer(BaseLinksSerializer, serializers.Serializer):
     dashboard = VersionDashboardURLsSerializer(source="*")
 
     def get_documentation(self, obj):
-        return resolver.resolve_version(
+        return Resolver().resolve_version(
             project=obj.project,
             version=obj,
         )

readthedocs/core/resolver.py

@@ -1,4 +1,5 @@
 """URL resolver for documentation."""
+from functools import lru_cache
 from urllib.parse import urlunparse
 
 import structlog
@@ -169,6 +170,7 @@ def resolve_project(self, project, filename="/"):
         protocol = "https" if use_https else "http"
         return urlunparse((protocol, domain, filename, "", "", ""))
 
+    @lru_cache(maxsize=1)
     def _get_project_domain(
         self, project, external_version_slug=None, use_canonical_domain=True
     ):
@@ -178,6 +180,11 @@ def _get_project_domain(
         :param project: Project object
         :param bool use_canonical_domain: If `True` use its canonical custom domain if available.
         :returns: Tuple of ``(domain, use_https)``.
+
+        Note that we are using ``lru_cache`` decorator on this function.
+        This is useful when generating the flyout addons response since we call
+        ``resolver.resolve`` multi times for the same ``Project``.
+        This cache avoids hitting the DB to get the canonical custom domain over and over again.
         """
         use_https = settings.PUBLIC_DOMAIN_USES_HTTPS
         canonical_project, _ = self._get_canonical_project(project)
@@ -223,6 +230,15 @@ def resolve(
         external=None,
         **kwargs,
     ):
+        """
+        Resolve the URL of the project/version_slug/filename combination.
+
+        :param project: Project to resolve.
+        :param filename: exact filename the resulting URL should contain.
+        :param query_params: query string params the resulting URL should contain.
+        :param external: whether or not the resolved URL would be external (`*.readthedocs.build`).
+        :param kwargs: extra attributes to be passed to ``resolve_path``.
+        """
         version_slug = kwargs.get("version_slug")
 
         if version_slug is None:
@@ -351,9 +367,3 @@ def _fix_filename(self, filename):
     def _use_cname(self, project):
         """Test if to allow direct serving for project on CNAME."""
         return bool(get_feature(project, feature_type=TYPE_CNAME))
-
-
-resolver = Resolver()
-resolve_path = resolver.resolve_path
-resolve_domain = resolver.get_domain_without_protocol
-resolve = resolver.resolve

readthedocs/core/templatetags/core_tags.py

@@ -9,7 +9,7 @@
 from django.utils.safestring import mark_safe
 
 from readthedocs import __version__
-from readthedocs.core.resolver import resolve
+from readthedocs.core.resolver import Resolver
 from readthedocs.projects.models import Project
 
 register = template.Library()
@@ -49,7 +49,7 @@ def make_document_url(project, version=None, page="", path=""):
     if not project:
         return ""
     filename = path or page
-    return resolve(project=project, version_slug=version, filename=filename)
+    return Resolver().resolve(project=project, version_slug=version, filename=filename)
 
 
 @register.filter

readthedocs/embed/views.py

@@ -15,7 +15,7 @@
 from readthedocs.api.mixins import CDNCacheTagsMixin, EmbedAPIMixin
 from readthedocs.api.v2.permissions import IsAuthorizedToViewVersion
 from readthedocs.api.v3.permissions import HasEmbedAPIAccess
-from readthedocs.core.resolver import resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.embed.utils import clean_references, recurse_while_none
 from readthedocs.storage import build_media_storage
 
@@ -134,7 +134,7 @@ def get(self, request):
 def do_embed(*, project, version, doc=None, path=None, section=None, url=None):
     """Get the embed response from a document section."""
     if not url:
-        url = resolver.resolve_version(
+        url = Resolver().resolve_version(
             project=project,
             version=version,
             filename=path or doc,

readthedocs/projects/models.py

@@ -32,7 +32,7 @@
     STABLE,
 )
 from readthedocs.core.history import ExtraHistoricalRecords
-from readthedocs.core.resolver import resolve, resolve_domain, resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.core.utils import extract_valid_attributes_for_model, slugify
 from readthedocs.core.utils.url import unsafe_join_url_path
 from readthedocs.domains.querysets import DomainQueryset
@@ -112,7 +112,7 @@ def save(self, *args, **kwargs):
 
     # HACK
     def get_absolute_url(self):
-        return resolver.resolve_version(project=self.child)
+        return Resolver().resolve_version(project=self.child)
 
     @cached_property
     def subproject_prefix(self):
@@ -673,7 +673,7 @@ def get_docs_url(self, version_slug=None, lang_slug=None, external=False):
 
         ``external`` defaults False because we only link external versions in very specific places
         """
-        return resolve(
+        return Resolver().resolve(
             project=self,
             version_slug=version_slug,
             language=lang_slug,
@@ -845,7 +845,9 @@ def alias(self):
 
     def subdomain(self, use_canonical_domain=True):
         """Get project subdomain from resolver."""
-        return resolve_domain(self, use_canonical_domain=use_canonical_domain)
+        return Resolver().get_domain_without_protocol(
+            self, use_canonical_domain=use_canonical_domain
+        )
 
     def get_downloads(self):
         downloads = {}
@@ -1510,7 +1512,7 @@ class ImportedFile(models.Model):
     )
 
     def get_absolute_url(self):
-        return resolver.resolve_version(
+        return Resolver().resolve_version(
             project=self.project,
             version=self.version.slug,
             filename=self.path,
@@ -1657,7 +1659,7 @@ def get_payload(self, version, build, event):
         protocol = 'http' if settings.DEBUG else 'https'
         project_url = f'{protocol}://{settings.PRODUCTION_DOMAIN}{project.get_absolute_url()}'
         build_url = f'{protocol}://{settings.PRODUCTION_DOMAIN}{build.get_absolute_url()}'
-        build_docsurl = resolver.resolve_version(project, version=version)
+        build_docsurl = Resolver().resolve_version(project, version=version)
 
         # Remove timezone and microseconds from the date,
         # so it's more readable.

readthedocs/projects/views/mixins.py

@@ -5,7 +5,7 @@
 from celery import chain
 from django.shortcuts import get_object_or_404
 
-from readthedocs.core.resolver import resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.core.utils import prepare_build
 from readthedocs.projects.models import Project
 from readthedocs.projects.signals import project_import
@@ -77,6 +77,7 @@ def _get_subprojects_and_urls(self):
         if not subprojects.exists():
             return subprojects_and_urls
 
+        resolver = Resolver()
         main_domain = resolver.get_domain(project)
         parsed_main_domain = urlparse(main_domain)
 

readthedocs/projects/views/public.py

@@ -29,7 +29,7 @@
 from readthedocs.builds.views import BuildTriggerMixin
 from readthedocs.core.mixins import CDNCacheControlMixin
 from readthedocs.core.permissions import AdminPermission
-from readthedocs.core.resolver import resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.core.utils.extend import SettingsOverrideObject
 from readthedocs.projects.filters import ProjectVersionListFilterSet
 from readthedocs.projects.models import Project
@@ -135,7 +135,7 @@ def get_context_data(self, **kwargs):
             protocol=protocol,
         )
         context["site_url"] = "{url}?badge={version}".format(
-            url=resolver.resolve_version(project, version=default_version),
+            url=Resolver().resolve_version(project, version=default_version),
             version=default_version_slug,
         )
 

readthedocs/proxito/redirects.py

@@ -3,7 +3,7 @@
 import structlog
 from django.http import HttpResponseRedirect
 
-from readthedocs.core.resolver import resolver
+from readthedocs.core.resolver import Resolver
 from readthedocs.core.utils.url import unsafe_join_url_path
 from readthedocs.proxito.cache import cache_response
 from readthedocs.proxito.constants import RedirectType
@@ -62,7 +62,7 @@ def canonical_redirect(request, project, redirect_type, external_version_slug=No
     elif redirect_type == RedirectType.subproject_to_main_domain:
         # We need to get the subproject root in the domain of the main
         # project, and append the current path.
-        project_doc_prefix = resolver.get_subproject_url_prefix(
+        project_doc_prefix = Resolver().get_subproject_url_prefix(
             project=project,
             external_version_slug=external_version_slug,
         )