Skip to content

Commit

Permalink
feat: meilisearch backend for notes search (openedx#444)
Browse files Browse the repository at this point in the history
* 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"
  • Loading branch information
regisb committed Nov 14, 2024
1 parent cee577c commit d4e5591
Show file tree
Hide file tree
Showing 19 changed files with 1,167 additions and 660 deletions.
41 changes: 28 additions & 13 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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"
17 changes: 15 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
***********************
Expand Down
96 changes: 96 additions & 0 deletions notesapi/v1/tests/test_meilisearch.py
Original file line number Diff line number Diff line change
@@ -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]
8 changes: 6 additions & 2 deletions notesapi/v1/urls.py
Original file line number Diff line number Diff line change
@@ -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'),
Expand All @@ -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'
),
]
Loading

0 comments on commit d4e5591

Please sign in to comment.