Refactor search view to make use of permission_classes

This is the same as #6133
but for the search view.

This allow us to override `get_all_projects` in .com
to filter projects using the auth backends.

And make use of the permission class from .com that makes use of the
auth backends.

This doesn't break the current search endpoint in .com
since it filters the versions using `public(user=self.request.user)`

This is required to proxy search on .com

Author: stsewd

readthedocs/api/v2/permissions.py

@@ -91,7 +91,7 @@ class IsAuthorizedToViewVersion(permissions.BasePermission):
     """
     Checks if the user from the request has permissions to see the version.
 
-    This permission class used in the FooterHTML view.
+    This permission class used in the FooterHTML and PageSearchAPIView views.
 
     .. note::
 

readthedocs/search/api.py

@@ -1,16 +1,18 @@
 import itertools
 import logging
 
+from django.shortcuts import get_object_or_404
 from django.utils import timezone
 from rest_framework import generics, serializers
 from rest_framework.exceptions import ValidationError
 from rest_framework.pagination import PageNumberPagination
 
-from readthedocs.projects.models import HTMLFile
+from readthedocs.api.v2.permissions import IsAuthorizedToViewVersion
+from readthedocs.builds.models import Version
+from readthedocs.projects.models import HTMLFile, Project
 from readthedocs.search import tasks, utils
 from readthedocs.search.faceted_search import PageSearch
 
-
 log = logging.getLogger(__name__)
 
 
@@ -60,11 +62,50 @@ def get_inner_hits(self, obj):
 
 class PageSearchAPIView(generics.ListAPIView):
 
-    """Main entry point to perform a search using Elasticsearch."""
+    """
+    Main entry point to perform a search using Elasticsearch.
+
+    Required query params:
+    - q (search term)
+    - project
+    - version
+
+    .. note::
+
+       The methods `_get_project` and `_get_version`
+       are called many times, so a basic cache is implemented.
+    """
 
+    permission_classes = [IsAuthorizedToViewVersion]
     pagination_class = SearchPagination
     serializer_class = PageSearchSerializer
 
+    def _get_project(self):
+        cache_key = '_cached_project'
+        project = getattr(self, cache_key, None)
+
+        if not project:
+            project_slug = self.request.GET.get('project', None)
+            project = get_object_or_404(Project, slug=project_slug)
+            setattr(self, cache_key, project)
+
+        return project
+
+    def _get_version(self):
+        cache_key = '_cached_version'
+        version = getattr(self, cache_key, None)
+
+        if not version:
+            version_slug = self.request.GET.get('version', None)
+            project = self._get_project()
+            version = get_object_or_404(
+                project.versions.all(),
+                slug=version_slug,
+            )
+            setattr(self, cache_key, version)
+
+        return version
+
     def get_queryset(self):
         """
         Return Elasticsearch DSL Search object instead of Django Queryset.
@@ -78,13 +119,7 @@ def get_queryset(self):
         query = self.request.query_params.get('q', '')
         kwargs = {'filter_by_user': False, 'filters': {}}
         kwargs['filters']['project'] = [p.slug for p in self.get_all_projects()]
-        kwargs['filters']['version'] = self.request.query_params.get('version')
-        if not kwargs['filters']['project']:
-            log.info("Unable to find a project to search")
-            return HTMLFile.objects.none()
-        if not kwargs['filters']['version']:
-            log.info("Unable to find a version to search")
-            return HTMLFile.objects.none()
+        kwargs['filters']['version'] = self._get_version().slug
         user = self.request.user
         queryset = PageSearch(
             query=query, user=user, **kwargs
@@ -120,17 +155,24 @@ def get_all_projects(self):
         """
         Return a list containing the project itself and all its subprojects.
 
-        The project slug is retrieved from ``project`` query param.
-
         :rtype: list
-
-        :raises: Http404 if project is not found
         """
-        project_slug = self.request.query_params.get('project')
-        version_slug = self.request.query_params.get('version')
-        all_projects = utils.get_project_list_or_404(
-            project_slug=project_slug, user=self.request.user, version_slug=version_slug,
+        main_version = self._get_version()
+        main_project = self._get_project()
+
+        subprojects = Project.objects.filter(
+            superprojects__parent_id=main_project.id,
         )
+        all_projects = []
+        for project in list(subprojects) + [main_project]:
+            version = (
+                Version.objects
+                .public(user=self.request.user, project=project)
+                .filter(slug=main_version.slug)
+                .first()
+            )
+            if version:
+                all_projects.append(version.project)
         return all_projects
 
     def get_all_projects_url(self):
@@ -151,7 +193,7 @@ def get_all_projects_url(self):
         :rtype: dict
         """
         all_projects = self.get_all_projects()
-        version_slug = self.request.query_params.get('version')
+        version_slug = self._get_version().slug
         projects_url = {}
         for project in all_projects:
             projects_url[project.slug] = project.get_docs_url(version_slug=version_slug)
@@ -162,8 +204,8 @@ def list(self, request, *args, **kwargs):
 
         response = super().list(request, *args, **kwargs)
 
-        project_slug = self.request.query_params.get('project', None)
-        version_slug = self.request.query_params.get('version', None)
+        project_slug = self._get_project().slug
+        version_slug = self._get_version().slug
         total_results = response.data.get('count', 0)
         time = timezone.now()
 

readthedocs/search/tests/test_api.py

@@ -200,11 +200,17 @@ def test_doc_search_pagination(self, api_client, project):
         assert len(resp.data['results']) == 5
 
     def test_doc_search_without_parameters(self, api_client, project):
-        """Hitting Document Search endpoint without query parameters should return error"""
+        """Hitting Document Search endpoint without project and version should return 404."""
         resp = self.get_search(api_client, {})
+        assert resp.status_code == 404
+
+    def test_doc_search_without_query(self, api_client, project):
+        """Hitting Document Search endpoint without a query should return error."""
+        resp = self.get_search(
+            api_client, {'project': project.slug, 'version': project.versions.first().slug})
         assert resp.status_code == 400
         # Check error message is there
-        assert sorted(['q', 'project', 'version']) == sorted(resp.data.keys())
+        assert 'q' in resp.data.keys()
 
     def test_doc_search_subprojects(self, api_client, all_projects):
         """Test Document search return results from subprojects also"""
@@ -256,7 +262,4 @@ def test_doc_search_unexisting_version(self, api_client, project):
             'version': version,
         }
         resp = self.get_search(api_client, search_params)
-        assert resp.status_code == 200
-
-        data = resp.data['results']
-        assert len(data) == 0
+        assert resp.status_code == 404

readthedocs/search/utils.py

@@ -82,27 +82,6 @@ def remove_indexed_files(model, project_slug, version_slug=None, build_id=None):
         log.exception('Unable to delete a subset of files. Continuing.')
 
 
-# TODO: Rewrite all the views using this in Class Based View,
-# and move this function to a mixin
-def get_project_list_or_404(project_slug, user, version_slug=None):
-    """
-    Return list of project and its subprojects.
-
-    It filters by Version privacy instead of Project privacy,
-    so we can support public versions on private projects.
-    """
-    project_list = []
-    main_project = get_object_or_404(Project, slug=project_slug)
-    subprojects = Project.objects.filter(superprojects__parent_id=main_project.id)
-    for project in list(subprojects) + [main_project]:
-        version = Version.internal.public(user).filter(
-            project__slug=project.slug, slug=version_slug
-        )
-        if version.exists():
-            project_list.append(version.first().project)
-    return project_list
-
-
 def _get_index(indices, index_name):
     """
     Get Index from all the indices.