Search: refactor api view

Mainly this was done to allow us support searching on
different versions of subprojects.
Currently, we assume we search the same version for all subprojects.

- _get_all_projects_data and _get_all_projects
  were merged into just one method.
  And instead of returning a list of projects we return a dictionary of project slugs mapped to a VersionData object.
- There is some duplication in .com.
  `_has_permission` and `_get_subproject_versions_queryset`
  are overridden in .com.
- ProjectData was renamed to VersionData since it has more attributes
  related to a version than a project.

Author: stsewd

readthedocs/search/api.py

@@ -15,7 +15,7 @@
 from readthedocs.search import tasks
 from readthedocs.search.faceted_search import PageSearch
 
-from .serializers import PageSearchSerializer
+from .serializers import PageSearchSerializer, VersionData
 
 log = logging.getLogger(__name__)
 
@@ -185,67 +185,82 @@ def _validate_query_params(self):
 
     def _get_all_projects_data(self):
         """
-        Return a dict containing the project slug and its version URL and version's doctype.
-
-        The dictionary contains the project and its subprojects. Each project's
-        slug is used as a key and a tuple with the documentation URL and doctype
-        from the version. Example:
-
-        {
-            "requests": (
-                "https://requests.readthedocs.io/en/latest/",
-                "sphinx",
-            ),
-            "requests-oauth": (
-                "https://requests-oauth.readthedocs.io/en/latest/",
-                "sphinx_htmldir",
-            ),
-        }
+        Return a dictionary of the project itself and all its subprojects.
 
-        :rtype: dict
-        """
-        all_projects = self._get_all_projects()
-        version_slug = self._get_version().slug
-        project_urls = {}
-        for project in all_projects:
-            project_urls[project.slug] = project.get_docs_url(version_slug=version_slug)
-
-        versions_doctype = (
-            Version.objects
-            .filter(project__slug__in=project_urls.keys(), slug=version_slug)
-            .values_list('project__slug', 'documentation_type')
-        )
+        Example:
 
-        projects_data = {
-            project_slug: (project_urls[project_slug], doctype)
-            for project_slug, doctype in versions_doctype
-        }
-        return projects_data
+        .. code::
 
-    def _get_all_projects(self):
-        """
-        Returns a list of the project itself and all its subprojects the user has permissions over.
+           {
+               "requests": VersionData(
+                   "latest",
+                   "sphinx",
+                   "https://requests.readthedocs.io/en/latest/",
+               ),
+               "requests-oauth": VersionData(
+                   "latest",
+                   "sphinx_htmldir",
+                   "https://requests-oauth.readthedocs.io/en/latest/",
+               ),
+           }
 
-        :rtype: list
+        .. note:: The response is cached into the instance.
+
+        :rtype: A dictionary of project slugs mapped to a `VersionData` object.
         """
+        cache_key = '__cached_projects_data'
+        projects_data = getattr(self, cache_key, None)
+        if projects_data is not None:
+            return projects_data
+
         main_version = self._get_version()
         main_project = self._get_project()
 
-        all_projects = [main_project]
+        projects_data = {
+            main_project.slug: VersionData(
+                slug=main_version.slug,
+                doctype=main_version.documentation_type,
+                docs_url=main_project.get_docs_url(version_slug=main_version.slug),
+            )
+        }
 
         subprojects = Project.objects.filter(
             superprojects__parent_id=main_project.id,
         )
         for project in subprojects:
-            version = (
-                Version.internal
-                .public(user=self.request.user, project=project, include_hidden=False)
-                .filter(slug=main_version.slug)
-                .first()
+            version = self._get_subproject_version(
+                version_slug=main_version.slug,
+                subproject=project,
             )
-            if version:
-                all_projects.append(version.project)
-        return all_projects
+            if version and self._has_permission(self.request.user, version):
+                url = project.get_docs_url(version_slug=version.slug)
+                projects_data[project.slug] = VersionData(
+                    slug=version.slug,
+                    doctype=version.documentation_type,
+                    docs_url=url,
+                )
+
+        setattr(self, cache_key, projects_data)
+        return projects_data
+
+    def _get_subproject_version(self, version_slug, subproject):
+        """Get a version from the subproject."""
+        return (
+            Version.internal
+            .public(user=self.request.user, project=subproject, include_hidden=False)
+            .filter(slug=version_slug)
+            .first()
+        )
+
+    def _has_permission(self, user, version):
+        """
+        Check if `user` is authorized to access `version`.
+
+        The queryset from `_get_subproject_version` already filters public
+        projects. This is mainly to be overriden in .com to make use of
+        the auth backends in the proxied API.
+        """
+        return True
 
     def _record_query(self, response):
         project_slug = self._get_project().slug
@@ -276,7 +291,7 @@ def get_queryset(self):
            is compatible with DRF's paginator.
         """
         filters = {}
-        filters['project'] = [p.slug for p in self._get_all_projects()]
+        filters['project'] = list(self._get_all_projects_data().keys())
         filters['version'] = self._get_version().slug
 
         # Check to avoid searching all projects in case these filters are empty.

readthedocs/search/serializers.py

@@ -20,8 +20,8 @@
 from readthedocs.projects.models import Project
 
 
-# Structure used for storing cached data of a project mostly.
-ProjectData = namedtuple('ProjectData', ['docs_url', 'version_doctype'])
+# Structure used for storing cached data of a version mostly.
+VersionData = namedtuple('VersionData', ['slug', 'docs_url', 'doctype'])
 
 
 class ProjectHighlightSerializer(serializers.Serializer):
@@ -89,14 +89,14 @@ def _get_full_path(self, obj):
         it's cached into ``project_data``.
         """
         # First try to build the URL from the context.
-        project_data = self.context.get('projects_data', {}).get(obj.project)
-        if project_data:
-            docs_url, doctype = project_data
+        version_data = self.context.get('projects_data', {}).get(obj.project)
+        if version_data:
+            docs_url = version_data.docs_url
             path = obj.full_path
 
             # Generate an appropriate link for the doctypes that use htmldir,
             # and always end it with / so it goes directly to proxito.
-            if doctype in {SPHINX_HTMLDIR, MKDOCS}:
+            if version_data.doctype in {SPHINX_HTMLDIR, MKDOCS}:
                 path = re.sub('(^|/)index.html$', '/', path)
 
             return docs_url.rstrip('/') + '/' + path.lstrip('/')
@@ -107,7 +107,11 @@ def _get_full_path(self, obj):
             docs_url = project.get_docs_url(version_slug=obj.version)
             # cache the project URL
             projects_data = self.context.setdefault('projects_data', {})
-            projects_data[obj.project] = ProjectData(docs_url, '')
+            projects_data[obj.project] = VersionData(
+                slug=obj.version,
+                docs_url=docs_url,
+                doctype=None,
+            )
             return docs_url + obj.full_path
 
         return None

readthedocs/search/tests/test_api.py

@@ -293,7 +293,7 @@ def test_doc_search_unexisting_version(self, api_client, project):
         resp = self.get_search(api_client, search_params)
         assert resp.status_code == 404
 
-    @mock.patch.object(PageSearchAPIView, '_get_all_projects', list)
+    @mock.patch.object(PageSearchAPIView, '_get_all_projects_data', dict)
     def test_get_all_projects_returns_empty_results(self, api_client, project):
         """If there is a case where `_get_all_projects` returns empty, we could be querying all projects."""
 

readthedocs/search/views.py

@@ -15,8 +15,8 @@
 
 from .serializers import (
     PageSearchSerializer,
-    ProjectData,
     ProjectSearchSerializer,
+    VersionData,
 )
 
 log = logging.getLogger(__name__)
@@ -63,7 +63,11 @@ def _get_project_data(self, project, version_slug):
         )
         docs_url = project.get_docs_url(version_slug=version_slug)
         project_data = {
-            project.slug: ProjectData(docs_url, version_doctype)
+            project.slug: VersionData(
+                slug=version_slug,
+                docs_url=docs_url,
+                doctype=version_doctype,
+            )
         }
         return project_data