feat: meilisearch backend for notes search (#444)

* feat: introduce "make compile-requirement" target

This is convenient to compile dependencies without upgrading them.

* chore: simplify tox/make test commands

This makes it possible to run the make commands directly without going
through tox (though tox targets keep working, of course).

* chore: more convenient unit test running

Previously, it was not possible to run unit tests locally without
manually creating mysql & elasticsearch containers. Here, we create a
`make pytest` target that automatically starts the required containers.

* 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.

* feat: meilisearch backend for notes search

This is a very simple and basic backend. It is based on Django signals,
just like the Elasticsearch backend. But it is much simpler, in the
sense that there are just two signals: one for saving documents and one
for deletion.

This backend is limited, in the sense that it does not support
highlighting -- but that's probably not such a big deal.

To start using this backend, define the following settings:

	ES_DISABLED = True
	MEILISEARCH_ENABLED = True
	MEILISEARCH_URL = "http://meilisearch:7700"
	MEILISEARCH_API_KEY = "s3cr3t"
	MEILISEARCH_INDEX = "tutor_student_notes"

Makefile

@@ -3,15 +3,23 @@ PACKAGES = notesserver notesapi
 
 validate: test.requirements test
 
+pytest: test-start-services test test-stop-services
+
 test: clean
 	python -Wd -m pytest
 
+test-start-services:
+	docker compose -f notesserver/docker-compose.test.yml --project-name=edxnotesapi_test up -d --remove-orphans
+
+test-stop-services:
+	docker compose -f notesserver/docker-compose.test.yml --project-name=edxnotesapi_test stop
+
 pii_check: test.requirements pii_clean
-	code_annotations django_find_annotations --config_file .pii_annotations.yml \
+	DJANGO_SETTINGS_MODULE=notesserver.settings.test code_annotations django_find_annotations --config_file .pii_annotations.yml \
 		--lint --report --coverage
 
 check_keywords: ## Scan the Django models in all installed apps in this project for restricted field names
-	python manage.py check_reserved_keywords --override_file db_keyword_overrides.yml
+	DJANGO_SETTINGS_MODULE=notesserver.settings.test python manage.py check_reserved_keywords --override_file db_keyword_overrides.yml
 
 run:
 	./manage.py runserver 0.0.0.0:8120
@@ -26,9 +34,13 @@ pii_clean:
 	rm -rf pii_report
 	mkdir -p pii_report
 
-quality:
-	pycodestyle --config=.pycodestyle  $(PACKAGES)
-	pylint $(PACKAGES)
+quality: pycodestyle pylint
+
+pycodestyle:
+	pycodestyle --config=.pycodestyle $(PACKAGES)
+
+pylint:
+	DJANGO_SETTINGS_MODULE=notesserver.settings.test pylint $(PACKAGES)
 
 diff-coverage:
 	diff-cover build/coverage/coverage.xml --html-report build/coverage/diff_cover.html
@@ -59,18 +71,21 @@ develop: requirements test.requirements
 piptools: ## install pinned version of pip-compile and pip-sync
 	pip install -r requirements/pip-tools.txt
 
-upgrade: export CUSTOM_COMPILE_COMMAND=make upgrade
-upgrade: piptools ## update the requirements/*.txt files with the latest packages satisfying requirements/*.in
+compile-requirements: export CUSTOM_COMPILE_COMMAND=make upgrade
+compile-requirements: piptools ## Re-compile *.in requirements to *.txt (without upgrading)
 	# Make sure to compile files after any other files they include!
-	pip-compile --upgrade --rebuild --allow-unsafe -o requirements/pip.txt requirements/pip.in
-	pip-compile --upgrade -o requirements/pip-tools.txt requirements/pip-tools.in
+	pip-compile ${COMPILE_OPTS} --rebuild --allow-unsafe -o requirements/pip.txt requirements/pip.in
+	pip-compile ${COMPILE_OPTS} -o requirements/pip-tools.txt requirements/pip-tools.in
 	pip install -qr requirements/pip.txt
 	pip install -qr requirements/pip-tools.txt
-	pip-compile --upgrade --allow-unsafe -o requirements/base.txt requirements/base.in
-	pip-compile --upgrade --allow-unsafe -o requirements/test.txt requirements/test.in
-	pip-compile --upgrade --allow-unsafe -o requirements/ci.txt requirements/ci.in
-	pip-compile --upgrade --allow-unsafe -o requirements/quality.txt requirements/quality.in
+	pip-compile ${COMPILE_OPTS} --allow-unsafe -o requirements/base.txt requirements/base.in
+	pip-compile ${COMPILE_OPTS} --allow-unsafe -o requirements/test.txt requirements/test.in
+	pip-compile ${COMPILE_OPTS} --allow-unsafe -o requirements/ci.txt requirements/ci.in
+	pip-compile ${COMPILE_OPTS} --allow-unsafe -o requirements/quality.txt requirements/quality.in
 	# Let tox control the Django version for tests
 	grep -e "^django==" requirements/base.txt > requirements/django.txt
 	sed '/^[dD]jango==/d' requirements/test.txt > requirements/test.tmp
 	mv requirements/test.tmp requirements/test.txt
+
+upgrade: ## update the requirements/*.txt files with the latest packages satisfying requirements/*.in
+	$(MAKE) compile-requirements COMPILE_OPTS="--upgrade"

README.rst

@@ -47,8 +47,21 @@ Configuration
 Running Tests
 *************
 
-Run ``make validate`` install the requirements, run the tests, and run
-lint.
+Install requirements::
+
+   make test.requirements
+
+Start mysql/elasticsearch services::
+
+   make test-start-services
+
+Run unit tests::
+
+   make test
+
+Run quality checks::
+
+   make quality
 
 How To Resync The Index
 ***********************

notesapi/v1/tests/test_meilisearch.py

@@ -0,0 +1,96 @@
+from unittest.mock import Mock, patch
+
+from django.test import TestCase
+
+from notesapi.v1.models import Note
+from notesapi.v1.views import meilisearch
+
+
+class MeilisearchTest(TestCase):
+
+    @classmethod
+    def setUpClass(cls):
+        super().setUpClass()
+        meilisearch.connect_signals()
+
+    @classmethod
+    def tearDownClass(cls):
+        super().tearDownClass()
+        meilisearch.disconnect_signals()
+
+    def setUp(self):
+        self.enterContext(
+            patch.object(meilisearch.Client, "meilisearch_client", Mock())
+        )
+        self.enterContext(patch.object(meilisearch.Client, "meilisearch_index", Mock()))
+
+    @property
+    def note_dict(self):
+        return {
+            "user": "test_user_id",
+            "usage_id": "i4x://org/course/html/52aa9816425a4ce98a07625b8cb70811",
+            "course_id": "org/course/run",
+            "text": "test note text",
+            "quote": "test note quote",
+            "ranges": [
+                {
+                    "start": "/p[1]",
+                    "end": "/p[1]",
+                    "startOffset": 0,
+                    "endOffset": 10,
+                }
+            ],
+            "tags": ["apple", "pear"],
+        }
+
+    def test_save_delete_note(self):
+        note = Note.create(self.note_dict)
+        note.save()
+        note_id = note.id
+
+        meilisearch.Client.meilisearch_index.add_documents.assert_called_with(
+            [
+                {
+                    "id": note_id,
+                    "user_id": "test_user_id",
+                    "course_id": "org/course/run",
+                    "text": "test note text",
+                }
+            ]
+        )
+
+        note.delete()
+        meilisearch.Client.meilisearch_index.delete_document.assert_called_with(note_id)
+
+    def test_get_queryset_no_result(self):
+        queryset = meilisearch.AnnotationSearchView().get_queryset()
+        assert not queryset.all()
+
+    def test_get_queryset_one_match(self):
+        note1 = Note.create(self.note_dict)
+        note2 = Note.create(self.note_dict)
+        note1.save()
+        note2.save()
+        view = meilisearch.AnnotationSearchView()
+        view.params = {
+            "text": "dummy text",
+            "user": "someuser",
+            "course_id": "course/id",
+            "page_size": 10,
+            "page": 2,
+        }
+        with patch.object(
+            meilisearch.Client.meilisearch_index,
+            "search",
+            Mock(return_value={"hits": [{"id": note2.id}]}),
+        ) as mock_search:
+            queryset = view.get_queryset()
+            mock_search.assert_called_once_with(
+                "dummy text",
+                {
+                    "offset": 10,
+                    "limit": 10,
+                    "filter": ["user_id = 'someuser'", "course_id = 'course/id'"],
+                },
+            )
+            assert [note2.id] == [note.id for note in queryset]

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_annotation_search_view_class)
 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_annotation_search_view_class().as_view(),
+        name='annotations_search'
+    ),
 ]