diff --git a/core/external_search.py b/core/external_search.py index 55f7b2ff76..518dbd7092 100644 --- a/core/external_search.py +++ b/core/external_search.py @@ -2807,11 +2807,11 @@ def on_completely_finished(self): target.finish() def run_once_and_update_timestamp(self): - try: - result = super().run_once_and_update_timestamp() - return result - finally: - self.on_completely_finished() + # We do not catch exceptions here, so that the on_completely finished should not run + # if there was a runtime error + result = super().run_once_and_update_timestamp() + self.on_completely_finished() + return result def process_batch(self, works) -> List[Work | CoverageFailure]: target: SearchDocumentReceiverType = self.migration or self.receiver diff --git a/core/search/revision.py b/core/search/revision.py index d5222fd67c..4bfd539011 100644 --- a/core/search/revision.py +++ b/core/search/revision.py @@ -15,6 +15,8 @@ class SearchSchemaRevision(ABC): SEARCH_VERSION: int def __init__(self): + if self.SEARCH_VERSION is None: + raise ValueError("The SEARCH_VERSION must be defined with an integer value") self._version = self.SEARCH_VERSION @abstractmethod diff --git a/core/search/revision_directory.py b/core/search/revision_directory.py index 0348daae66..6adbdb01bf 100644 --- a/core/search/revision_directory.py +++ b/core/search/revision_directory.py @@ -4,13 +4,24 @@ from core.search.revision import SearchSchemaRevision from core.search.v5 import SearchV5 +REVISIONS = [SearchV5()] + class SearchRevisionDirectory: """A directory of the supported search index schemas.""" @staticmethod def _create_revisions() -> Mapping[int, SearchSchemaRevision]: - return {r.version: r for r in [SearchV5()]} + numbers = set() + revisions = {} + for r in REVISIONS: + if r.version in numbers: + raise ValueError( + f"Revision version {r.version} is defined multiple times" + ) + numbers.add(r.version) + revisions[r.version] = r + return revisions def __init__(self, available: Mapping[int, SearchSchemaRevision]): self._available = available diff --git a/tests/core/search/test_search_revision_directory.py b/tests/core/search/test_search_revision_directory.py new file mode 100644 index 0000000000..b5c826365e --- /dev/null +++ b/tests/core/search/test_search_revision_directory.py @@ -0,0 +1,50 @@ +from unittest import mock + +import pytest + +from core.search.document import SearchMappingDocument +from core.search.revision import SearchSchemaRevision +from core.search.revision_directory import SearchRevisionDirectory + + +class AnyNumberRevision(SearchSchemaRevision): + def __init__(self, number): + self.SEARCH_VERSION = number + super().__init__() + + def mapping_document(self) -> SearchMappingDocument: + return SearchMappingDocument() + + +class TestSearchRevisionDirectory: + def test_create(self): + """Also tests _create_revisions""" + with mock.patch("core.search.revision_directory.REVISIONS", new=[]): + assert SearchRevisionDirectory.create().available == {} + + with mock.patch( + "core.search.revision_directory.REVISIONS", + new=[AnyNumberRevision(1), AnyNumberRevision(2)], + ): + assert list(SearchRevisionDirectory.create().available.keys()) == [1, 2] + + with mock.patch( + "core.search.revision_directory.REVISIONS", + new=[AnyNumberRevision(1), AnyNumberRevision(1)], + ): + with pytest.raises(ValueError) as raised: + SearchRevisionDirectory.create() + assert str(raised.value) == "Revision version 1 is defined multiple times" + + def test_highest(self): + with mock.patch( + "core.search.revision_directory.REVISIONS", + new=[AnyNumberRevision(1), AnyNumberRevision(2)], + ): + assert SearchRevisionDirectory.create().highest().version == 2 + + with mock.patch( + "core.search.revision_directory.REVISIONS", + new=[AnyNumberRevision(17), AnyNumberRevision(2)], + ): + assert SearchRevisionDirectory.create().highest().version == 17