Remove check_dependencies() from the search protocol. #325

src/reader/_search.py

@@ -160,7 +160,7 @@ def strip_html(text: SQLiteType) -> SQLiteType:
         return strip_html(text)
 
     @wrap_exceptions(SearchError)
-    def check_dependencies(self) -> None:
+    def check_update_dependencies(self) -> None:
         # Only update() requires these, so we don't check in __init__().
         # ... except json_each(), which is used in one of the triggers
         # (which is acceptable, we're trying to fail early for *most* cases).
@@ -173,13 +173,14 @@ def check_dependencies(self) -> None:
 
     @wrap_exceptions(SearchError)
     def enable(self) -> None:
+        self.check_update_dependencies()
         try:
             with ddl_transaction(self.get_db()) as db:
                 self._enable(db)
         except sqlite3.OperationalError as e:
             if "table entries_search already exists" in str(e).lower():
                 return
-            raise
+            raise  # pragma: no cover
 
     @classmethod
     def _enable(cls, db: sqlite3.Connection) -> None:
@@ -453,7 +454,7 @@ def _is_enabled(db: sqlite3.Connection) -> bool:
     @wrap_exceptions(SearchError)
     @wrap_search_exceptions()
     def update(self) -> None:
-        self.check_dependencies()
+        self.check_update_dependencies()
         self._delete_from_search()
         self._insert_into_search()
 

src/reader/_types.py

@@ -1096,11 +1096,24 @@ class SearchType(Protocol):  # pragma: no cover
 
     Any method can raise :exc:`.SearchError`.
 
+    There are two sets of methods that may be called at different times:
+
+    management methods
+        :meth:`enable` :meth:`disable` :meth:`is_enabled` :meth:`update`
+
+    read-only methods
+        :meth:`search_entries` :meth:`search_entry_counts`
+
     """
 
     def enable(self) -> None:
         """Called by :meth:`.Reader.enable_search`.
 
+        A no-op and reasonably fast if search is already enabled.
+
+        Checks if all dependencies needed for :meth:`update` are available,
+        raises :exc:`.SearchError` if not.
+
         Raises:
             StorageError
 
@@ -1112,17 +1125,21 @@ def disable(self) -> None:
     def is_enabled(self) -> bool:
         """Called by :meth:`.Reader.is_search_enabled`.
 
+        Not called otherwise.
+
         Returns:
             Whether search is enabled or not.
 
         """
 
-    def check_dependencies(self) -> None:
-        """Raise :exc:`.SearchError` if any required dependencies are missing.
+    def update(self) -> None:
+        """Called by :meth:`.Reader.update_search`.
 
-        .. admonition:: Unstable
+        Should not enable search automatically (handled by :class:`.Reader`).
 
-            This method may be removed in the future.
+        Raises:
+            SearchNotEnabledError
+            StorageError
 
         """
 
@@ -1172,12 +1189,3 @@ def search_entry_counts(
             StorageError
 
         """
-
-    def update(self) -> None:
-        """Called by :meth:`.Reader.update_search`.
-
-        Raises:
-            SearchNotEnabledError
-            StorageError
-
-        """

src/reader/core.py

@@ -267,7 +267,6 @@ def make_reader(
         search = storage.make_search()
 
         if search_enabled is True:
-            search.check_dependencies()
             search.enable()
         elif search_enabled is False:
             search.disable()

tests/test_search.py

@@ -182,16 +182,22 @@ def test_invalid_search_query_error(storage, query, exc_type, call_method):
 
 
 def test_minimum_sqlite_version(storage, monkeypatch):
+    search = Search(storage)
+    search.enable()
+
     mock = MagicMock(wraps=require_version, side_effect=DBError('version'))
     monkeypatch.setattr('reader._search.require_version', mock)
 
-    search = Search(storage)
-    search.enable()
+    with pytest.raises(SearchError) as excinfo:
+        search.enable()
+    assert 'version' in excinfo.value.message
+    mock.assert_called_with(ANY, (3, 18))
+
+    mock.reset_mock()
 
     with pytest.raises(SearchError) as excinfo:
         search.update()
     assert 'version' in excinfo.value.message
-
     mock.assert_called_with(ANY, (3, 18))