Skip to content

Commit 4f197ff

Browse files
committed
fixup! feat: Adds a way to find the difference in Meiliseach state and come up with a migration plan and configuration plan depending on the state. This introduces a mechanism it or a drift engine which drill down the Meiliseach configuration and figures out what has changed:
1 parent b7dc62e commit 4f197ff

2 files changed

Lines changed: 63 additions & 51 deletions

File tree

openedx/core/djangoapps/content/search/api.py

Lines changed: 14 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
from functools import wraps
1313
from typing import Callable, Generator, cast # noqa: UP035
1414

15-
from attrs import define, field
15+
from attrs import define
1616
from django.conf import settings
1717
from django.contrib.auth import get_user_model
1818
from django.core.cache import cache
@@ -383,13 +383,13 @@ class IndexDrift:
383383
"""
384384

385385
exists: bool
386-
is_empty: bool | None = field(default=None) # None if index doesn't exist
387-
primary_key_correct: bool | None = field(default=None) # None if index doesn't exist
388-
distinct_attribute_match: bool | None = field(default=None)
389-
filterable_attributes_match: bool | None = field(default=None)
390-
searchable_attributes_match: bool | None = field(default=None)
391-
sortable_attributes_match: bool | None = field(default=None)
392-
ranking_rules_match: bool | None = field(default=None)
386+
is_empty: bool | None = None # None if index doesn't exist
387+
primary_key_correct: bool | None = None # None if index doesn't exist
388+
distinct_attribute_match: bool | None = None
389+
filterable_attributes_match: bool | None = None
390+
searchable_attributes_match: bool | None = None
391+
sortable_attributes_match: bool | None = None
392+
ranking_rules_match: bool | None = None
393393

394394
@property
395395
def is_settings_drifted(self) -> bool:
@@ -405,16 +405,6 @@ def is_settings_drifted(self) -> bool:
405405
)
406406
)
407407

408-
@property
409-
def is_drifted(self) -> bool:
410-
"""True if settings are drifted OR primary key is incorrect."""
411-
return self.is_settings_drifted or self.primary_key_correct is False
412-
413-
@property
414-
def is_configured(self) -> bool:
415-
"""True if all settings match AND primary key is correct."""
416-
return not self.is_drifted
417-
418408

419409
def _detect_index_drift(index_name: str) -> IndexDrift:
420410
"""
@@ -527,7 +517,7 @@ def reconcile_index(
527517
if not drift.exists:
528518
status_cb("Studio search index not found. Creating and configuring...")
529519
reset_index(status_cb)
530-
status_cb("Index created. Run ./manage.py cms reindex_studio to populate.")
520+
status_cb("Index created. Run './manage.py cms' reindex_studio to populate.")
531521
return
532522

533523
# CASE: Primary key mismatch (must recreate regardless of population state)
@@ -539,20 +529,20 @@ def reconcile_index(
539529
f"PRIMARY KEY MISMATCH on populated index '{STUDIO_INDEX_NAME}'. "
540530
"Index must be recreated (data loss is unavoidable for primary key changes)."
541531
)
542-
warn_cb("Dropping and recreating index. Repopulate with: ./manage.py cms reindex_studio")
532+
warn_cb("Dropping and recreating index. Repopulate with: './manage.py cms' reindex_studio")
543533
reset_index(status_cb)
544-
warn_cb("Index recreated empty. Run ./manage.py cms reindex_studio to repopulate.")
534+
warn_cb("Index recreated empty. Run './manage.py cms' reindex_studio to repopulate.")
545535
return
546536

547537
# CASE: Index empty
548538
if drift.is_empty:
549539
if drift.is_settings_drifted:
550540
status_cb("Empty index has drifted settings. Reconfiguring...")
551541
_apply_settings_with_waits(STUDIO_INDEX_NAME, status_cb)
552-
status_cb("Reconfigured. Run ./manage.py cms reindex_studio to populate.")
542+
status_cb("Reconfigured. Run './manage.py cms' reindex_studio to populate.")
553543
else:
554544
status_cb(
555-
"Index exists and is correctly configured but empty. Run ./manage.py cms reindex_studio to populate."
545+
"Index exists and is correctly configured but empty. Run './manage.py cms' reindex_studio to populate."
556546
)
557547
return
558548

@@ -573,7 +563,7 @@ def reconcile_index(
573563
_apply_settings_with_waits(STUDIO_INDEX_NAME, status_cb)
574564
warn_cb(
575565
"Settings applied. Meilisearch will re-index documents in the background. "
576-
"Consider running ./manage.py cms reindex_studio for a full rebuild "
566+
"Consider running './manage.py cms' reindex_studio for a full rebuild "
577567
"if search quality is affected."
578568
)
579569
else:

openedx/core/djangoapps/content/search/tests/test_reconcile.py

Lines changed: 49 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
"""
22
Tests for the Meilisearch index reconciliation logic.
33
"""
4+
45
from __future__ import annotations
56

67
from unittest.mock import MagicMock, Mock, patch
@@ -37,7 +38,7 @@
3738
class TestIndexDrift(TestCase):
3839
"""Tests for the IndexDrift dataclass."""
3940

40-
def test_not_drifted_when_all_match(self):
41+
def test_all_fields_match(self):
4142
drift = IndexDrift(
4243
exists=True,
4344
is_empty=False,
@@ -48,9 +49,15 @@ def test_not_drifted_when_all_match(self):
4849
sortable_attributes_match=True,
4950
ranking_rules_match=True,
5051
)
51-
assert not drift.is_drifted
52+
assert drift.exists is True
53+
assert drift.is_empty is False
54+
assert drift.primary_key_correct is True
55+
assert drift.distinct_attribute_match is True
56+
assert drift.filterable_attributes_match is True
57+
assert drift.searchable_attributes_match is True
58+
assert drift.sortable_attributes_match is True
59+
assert drift.ranking_rules_match is True
5260
assert not drift.is_settings_drifted
53-
assert drift.is_configured
5461

5562
def test_settings_drifted_when_one_setting_false(self):
5663
drift = IndexDrift(
@@ -63,11 +70,11 @@ def test_settings_drifted_when_one_setting_false(self):
6370
sortable_attributes_match=True,
6471
ranking_rules_match=True,
6572
)
73+
assert drift.primary_key_correct is True
74+
assert drift.filterable_attributes_match is False
6675
assert drift.is_settings_drifted
67-
assert drift.is_drifted
68-
assert not drift.is_configured
6976

70-
def test_drifted_when_primary_key_wrong(self):
77+
def test_primary_key_wrong_without_settings_drift(self):
7178
drift = IndexDrift(
7279
exists=True,
7380
is_empty=False,
@@ -78,16 +85,25 @@ def test_drifted_when_primary_key_wrong(self):
7885
sortable_attributes_match=True,
7986
ranking_rules_match=True,
8087
)
88+
assert drift.primary_key_correct is False
89+
assert drift.distinct_attribute_match is True
90+
assert drift.filterable_attributes_match is True
91+
assert drift.searchable_attributes_match is True
92+
assert drift.sortable_attributes_match is True
93+
assert drift.ranking_rules_match is True
8194
assert not drift.is_settings_drifted
82-
assert drift.is_drifted
83-
assert not drift.is_configured
8495

85-
def test_not_drifted_when_index_missing(self):
96+
def test_missing_index_leaves_optional_fields_unset(self):
8697
drift = IndexDrift(exists=False)
87-
# When index doesn't exist, settings fields are None, so is_settings_drifted is False
98+
assert drift.exists is False
99+
assert drift.is_empty is None
100+
assert drift.primary_key_correct is None
101+
assert drift.distinct_attribute_match is None
102+
assert drift.filterable_attributes_match is None
103+
assert drift.searchable_attributes_match is None
104+
assert drift.sortable_attributes_match is None
105+
assert drift.ranking_rules_match is None
88106
assert not drift.is_settings_drifted
89-
# But is_drifted checks primary_key_correct is None (not False), so also not drifted
90-
assert not drift.is_drifted
91107

92108
def test_multiple_settings_drifted(self):
93109
drift = IndexDrift(
@@ -100,8 +116,12 @@ def test_multiple_settings_drifted(self):
100116
sortable_attributes_match=False,
101117
ranking_rules_match=True,
102118
)
119+
assert drift.distinct_attribute_match is False
120+
assert drift.filterable_attributes_match is False
121+
assert drift.searchable_attributes_match is True
122+
assert drift.sortable_attributes_match is False
123+
assert drift.ranking_rules_match is True
103124
assert drift.is_settings_drifted
104-
assert drift.is_drifted
105125

106126

107127
@skip_unless_cms
@@ -118,6 +138,7 @@ def setUp(self):
118138
def test_index_missing(self, mock_meilisearch):
119139
"""When the index doesn't exist, returns exists=False with all other fields None."""
120140
from meilisearch.errors import MeilisearchApiError
141+
121142
mock_meilisearch.return_value.get_index.side_effect = MeilisearchApiError(
122143
"Not found", Mock(text='{"code":"index_not_found"}')
123144
)
@@ -146,10 +167,14 @@ def test_all_settings_match(self, mock_meilisearch):
146167
drift = _detect_index_drift("test_index")
147168

148169
assert drift.exists
149-
assert not drift.is_empty
150-
assert drift.primary_key_correct
151-
assert not drift.is_drifted
152-
assert drift.is_configured
170+
assert drift.is_empty is False
171+
assert drift.primary_key_correct is True
172+
assert drift.distinct_attribute_match is True
173+
assert drift.filterable_attributes_match is True
174+
assert drift.searchable_attributes_match is True
175+
assert drift.sortable_attributes_match is True
176+
assert drift.ranking_rules_match is True
177+
assert not drift.is_settings_drifted
153178

154179
def test_filterable_attributes_mismatch(self, mock_meilisearch):
155180
"""Detects when filterable attributes differ."""
@@ -212,7 +237,8 @@ def test_filterable_attributes_order_independent(self, mock_meilisearch):
212237
assert drift.filterable_attributes_match is True
213238
assert drift.searchable_attributes_match is True
214239
assert drift.sortable_attributes_match is True
215-
assert not drift.is_drifted
240+
assert drift.primary_key_correct is True
241+
assert not drift.is_settings_drifted
216242

217243
def test_primary_key_mismatch(self, mock_meilisearch):
218244
"""Detects primary key mismatch."""
@@ -230,8 +256,9 @@ def test_primary_key_mismatch(self, mock_meilisearch):
230256

231257
drift = _detect_index_drift("test_index")
232258

259+
assert drift.exists is True
260+
assert drift.is_empty is False
233261
assert drift.primary_key_correct is False
234-
assert drift.is_drifted
235262
# Settings themselves are fine
236263
assert not drift.is_settings_drifted
237264

@@ -349,8 +376,7 @@ def test_index_empty_configured(self, mock_drift, mock_meilisearch):
349376
reconcile_index(status_cb=status_cb)
350377

351378
status_cb.assert_any_call(
352-
"Index exists and is correctly configured but empty. "
353-
"Run ./manage.py cms reindex_studio to populate."
379+
"Index exists and is correctly configured but empty. Run './manage.py cms' reindex_studio to populate."
354380
)
355381

356382
@patch("openedx.core.djangoapps.content.search.api._detect_index_drift")
@@ -461,9 +487,7 @@ def test_index_populated_wrong_pk(self, mock_reset, mock_drift, mock_meilisearch
461487

462488
mock_reset.assert_called_once()
463489
# Should warn about data loss
464-
warn_cb.assert_any_call(
465-
"Index recreated empty. Run ./manage.py cms reindex_studio to repopulate."
466-
)
490+
warn_cb.assert_any_call("Index recreated empty. Run './manage.py cms' reindex_studio to repopulate.")
467491

468492
@override_settings(MEILISEARCH_ENABLED=False)
469493
def test_meilisearch_disabled(self, mock_meilisearch):
@@ -553,9 +577,7 @@ def test_signal_connected(self, mock_meilisearch):
553577

554578
# Check that the handler is in the receivers
555579
receiver_funcs = [r[1]() for r in post_migrate.receivers if r[1]() is not None]
556-
assert handler_fn in receiver_funcs, (
557-
"handle_post_migrate should be connected to post_migrate signal"
558-
)
580+
assert handler_fn in receiver_funcs, "handle_post_migrate should be connected to post_migrate signal"
559581

560582

561583
@skip_unless_cms

0 commit comments

Comments
 (0)