chore: refactor views for better mysql/es separation

Instead of checking a boolean flag in multiple different places, we use
class inheritance. This makes it possible to later override the view and
implement our own using a different search backend, such as Meilisearch.

notesapi/v1/tests/test_views.py

@@ -1,4 +1,3 @@
-import sys
 import unittest
 from calendar import timegm
 from datetime import datetime, timedelta
@@ -176,7 +175,6 @@ def verify_list_view_pagination(
         """
         Verify pagination information for AnnotationListView
         """
-        total_annotations = total_annotations
         for i in range(total_annotations):
             self._create_annotation(text=f'annotation {i}')
 
@@ -207,7 +205,6 @@ def verify_search_view_pagination(
         """
         Verify pagination information for AnnotationSearchView
         """
-        total_annotations = total_annotations
         for i in range(total_annotations):
             self._create_annotation(text=f'annotation {i}')
 
@@ -370,15 +367,15 @@ def test_create_maximum_allowed(self):
         # if user tries to create note in a different course it should succeed
         kwargs = {'course_id': 'test-course-id-2'}
         response = self._create_annotation(**kwargs)
-        self.assertTrue('id' in response)
+        self.assertIn('id', response)
 
         # if another user to tries to create note in first course it should succeed
         token = get_id_token(TEST_OTHER_USER)
         self.client.credentials(HTTP_X_ANNOTATOR_AUTH_TOKEN=token)
         self.headers = {'user': TEST_OTHER_USER}
         kwargs = {'user': TEST_OTHER_USER}
         response = self._create_annotation(**kwargs)
-        self.assertTrue('id' in response)
+        self.assertIn('id', response)
 
     def test_read_all_no_annotations(self):
         """

notesapi/v1/urls.py

@@ -1,7 +1,7 @@
 from django.urls import path, re_path
 
 from notesapi.v1.views import (AnnotationDetailView, AnnotationListView,
-                               AnnotationRetireView, AnnotationSearchView)
+                               AnnotationRetireView, get_views_module)
 app_name = "notesapi.v1"
 urlpatterns = [
     path('annotations/', AnnotationListView.as_view(), name='annotations'),
@@ -11,5 +11,9 @@
         AnnotationDetailView.as_view(),
         name='annotations_detail'
     ),
-    path('search/', AnnotationSearchView.as_view(), name='annotations_search'),
+    path(
+        'search/',
+        get_views_module().AnnotationSearchView.as_view(),
+        name='annotations_search'
+    ),
 ]

notesapi/v1/views/__init__.py

@@ -0,0 +1,19 @@
+from django.conf import settings
+
+from .common import (
+    AnnotationDetailView,
+    AnnotationListView,
+    AnnotationRetireView,
+)
+
+from .exceptions import SearchViewRuntimeError
+
+def get_views_module():
+    """
+    Import views from either mysql or elasticsearch backend
+    """
+    if settings.ES_DISABLED:
+        from . import common as backend_module
+    else:
+        from . import elasticsearch as backend_module
+    return backend_module

notesapi/v1/views.py → notesapi/v1/views/common.py

@@ -12,21 +12,8 @@
 from rest_framework.views import APIView
 
 from notesapi.v1.models import Note
-from notesapi.v1.search_indexes.documents import NoteDocument
 from notesapi.v1.serializers import NoteSerializer
 
-if not settings.ES_DISABLED:
-    from elasticsearch_dsl import Search
-    from elasticsearch_dsl.connections import connections
-    from django_elasticsearch_dsl_drf.filter_backends import DefaultOrderingFilterBackend, HighlightBackend
-    from django_elasticsearch_dsl_drf.constants import (
-        LOOKUP_FILTER_TERM,
-        LOOKUP_QUERY_IN,
-        SEPARATOR_LOOKUP_COMPLEX_VALUE,
-    )
-    from notesapi.v1.search_indexes.paginators import NotesPagination as ESNotesPagination
-    from notesapi.v1.search_indexes.backends import CompoundSearchFilterBackend, FilteringFilterBackend
-    from notesapi.v1.search_indexes.serializers import NoteDocumentSerializer as NotesElasticSearchSerializer
 
 log = logging.getLogger(__name__)
 
@@ -126,99 +113,42 @@ class AnnotationSearchView(ListAPIView):
     params = {}
     query_params = {}
     search_with_usage_id = False
-    document = NoteDocument
     search_fields = ('text', 'tags')
-    filter_fields = {
-        'course_id': 'course_id',
-        'user': 'user',
-        'usage_id': {
-            'field': 'usage_id',
-            'lookups': [
-                LOOKUP_QUERY_IN,
-                LOOKUP_FILTER_TERM,
-            ],
-        },
-
-    }
-    highlight_fields = {
-        'text': {
-            'enabled': True,
-            'options': {
-                'pre_tags': ['{elasticsearch_highlight_start}'],
-                'post_tags': ['{elasticsearch_highlight_end}'],
-                'number_of_fragments': 0,
-            },
-        },
-        'tags': {
-            'enabled': True,
-            'options': {
-                'pre_tags': ['{elasticsearch_highlight_start}'],
-                'post_tags': ['{elasticsearch_highlight_end}'],
-                'number_of_fragments': 0,
-            },
-        },
-    }
     ordering = ('-updated',)
 
-    def __init__(self, *args, **kwargs):
-        self.initiate_es_specific_state_if_is_enabled()
-        super().__init__(*args, **kwargs)
-
-    def initiate_es_specific_state_if_is_enabled(self):
-        """
-        Initiates elasticsearch specific state if elasticsearch is enabled.
-
-        Should be called in the class `__init__` method.
-        """
-        if not settings.ES_DISABLED:
-            self.client = connections.get_connection(self.document._get_using())
-            self.index = self.document._index._name
-            self.mapping = self.document._doc_type.mapping.properties.name
-            self.search = Search(using=self.client, index=self.index, doc_type=self.document._doc_type.name)
 
     @property
-    def is_es_disabled(self):
+    def is_text_search(self):
         """
-        Predicate instance method.
-
-        Search in DB when ES is not available or there is no need to bother it
+        We identify text search by the presence of a "text" parameter. Subclasses may
+        want to have a different behaviour in such cases.
         """
-
-        return settings.ES_DISABLED or 'text' not in self.params
+        return "text" in self.params
 
     def get_queryset(self):
-        if self.is_es_disabled:
-            queryset = Note.objects.filter(**self.query_params).order_by('-updated')
-            if 'text' in self.params:
-                queryset = queryset.filter(
-                    Q(text__icontains=self.params['text']) | Q(tags__icontains=self.params['text'])
-                )
-        else:
-            queryset = self.search.query()
-            queryset.model = self.document.Django.model
-
+        queryset = Note.objects.filter(**self.query_params).order_by("-updated")
+        if "text" in self.params:
+            queryset = queryset.filter(
+                Q(text__icontains=self.params["text"])
+                | Q(tags__icontains=self.params["text"])
+            )
         return queryset
 
     def get_serializer_class(self):
         """
         Return the class to use for the serializer.
 
-        Defaults to using `NoteSerializer` if elasticsearch is disabled
-        or `NotesElasticSearchSerializer` if elasticsearch is enabled
+        Defaults to `NoteSerializer`.
         """
-
-        return NoteSerializer if self.is_es_disabled else NotesElasticSearchSerializer
+        return NoteSerializer
 
     @property
     def paginator(self):
         """
         The paginator instance associated with the view and used data source, or `None`.
         """
-        if not hasattr(self, '_paginator'):
-            if self.pagination_class is None:
-                self._paginator = None
-            else:
-                self._paginator = self.pagination_class() if self.is_es_disabled else ESNotesPagination()
+        if not hasattr(self, "_paginator"):
+            self._paginator = self.pagination_class() if self.pagination_class else None
 
         return self._paginator
 
@@ -229,20 +159,17 @@ def filter_queryset(self, queryset):
         Do not filter additionally if mysql db used or use `CompoundSearchFilterBackend`
         and `HighlightBackend` if elasticsearch is the data source.
         """
-        filter_backends = []
-        if not self.is_es_disabled:
-            filter_backends = [
-                FilteringFilterBackend,
-                CompoundSearchFilterBackend,
-                DefaultOrderingFilterBackend,
-            ]
-            if self.params.get('highlight'):
-                filter_backends.append(HighlightBackend)
-
+        filter_backends = self.get_filter_backends()
         for backend in filter_backends:
             queryset = backend().filter_queryset(self.request, queryset, view=self)
         return queryset
 
+    def get_filter_backends(self):
+        """
+        List of filter backends, each with a `filter_queryset` method.
+        """
+        return []
+
     def list(self, *args, **kwargs):
         """
         Returns list of students notes.
@@ -262,24 +189,18 @@ def build_query_params_state(self):
         """
         self.query_params = {}
         self.params = self.request.query_params.dict()
-        usage_ids = self.request.query_params.getlist('usage_id')
+        usage_ids = self.request.query_params.getlist("usage_id")
         if usage_ids:
             self.search_with_usage_id = True
-            if not self.is_es_disabled:
-                usage_ids = SEPARATOR_LOOKUP_COMPLEX_VALUE.join(usage_ids)
-
-            self.query_params['usage_id__in'] = usage_ids
+            self.query_params["usage_id__in"] = usage_ids
 
-        if 'course_id' in self.params:
-            self.query_params['course_id'] = self.params['course_id']
+        if "course_id" in self.params:
+            self.query_params["course_id"] = self.params["course_id"]
 
-        if 'user' in self.params:
-            if self.is_es_disabled:
-                self.query_params['user_id'] = self.params['user']
-            else:
-                self.query_params['user'] = self.params['user']
+        if "user" in self.params:
+            self.query_params["user_id"] = self.params["user"]
 
-    def get(self, *args, **kwargs):  # pylint: disable=unused-argument
+    def get(self, *args, **kwargs):
         """
         Search annotations in most appropriate storage
         """
@@ -299,10 +220,10 @@ def post(self, *args, **kwargs):  # pylint: disable=unused-argument
         Delete all annotations for a user.
         """
         params = self.request.data
-        if 'user' not in params:
+        if "user" not in params:
             return Response(status=status.HTTP_400_BAD_REQUEST)
 
-        Note.objects.filter(user_id=params['user']).delete()
+        Note.objects.filter(user_id=params["user"]).delete()
         return Response(status=status.HTTP_204_NO_CONTENT)
 
 
@@ -602,3 +523,15 @@ def delete(self, *args, **kwargs):  # pylint: disable=unused-argument
 
         # Annotation deleted successfully.
         return Response(status=status.HTTP_204_NO_CONTENT)
+
+def selftest():
+    """
+    No-op.
+    """
+    return {}
+
+def heartbeat():
+    """
+    No-op
+    """
+    return