fix(core): restore the page cache state once per fetch_schema call

src/osw/core.py

@@ -496,24 +496,36 @@ def fetch_schema(
         first = True
         last = False
         results = []
-        for schema_title in fetchSchemaParam.schema_title:
-            last = schema_title == fetchSchemaParam.schema_title[-1]
-            mode = fetchSchemaParam.mode
-            if not first:  # 'replace' makes only sense for the first schema
-                mode = "append"
-            res = self._fetch_schema(
-                OSW._FetchSchemaParam(
-                    schema_title=schema_title,
-                    mode=mode,
-                    final=last,
-                    generate_annotations=fetchSchemaParam.generate_annotations,
-                    generator_options=fetchSchemaParam.generator_options,
-                    offline_pages=fetchSchemaParam.offline_pages,
-                    result_model_path=fetchSchemaParam.result_model_path,
+        # the page cache is enabled once for the whole operation, because the same
+        # schema pages are read repeatedly while $refs are resolved. The state is
+        # taken and restored here and not per schema title: _fetch_schema would
+        # snapshot the state its own predecessor has already changed. The restore
+        # runs in a finally block so that an early return or an exception in
+        # _fetch_schema cannot leave the cache enabled for the rest of the process.
+        site_cache_state = self.site.get_cache_enabled()
+        self.site.enable_cache()
+        try:
+            for schema_title in fetchSchemaParam.schema_title:
+                last = schema_title == fetchSchemaParam.schema_title[-1]
+                mode = fetchSchemaParam.mode
+                if not first:  # 'replace' makes only sense for the first schema
+                    mode = "append"
+                res = self._fetch_schema(
+                    OSW._FetchSchemaParam(
+                        schema_title=schema_title,
+                        mode=mode,
+                        final=last,
+                        generate_annotations=fetchSchemaParam.generate_annotations,
+                        generator_options=fetchSchemaParam.generator_options,
+                        offline_pages=fetchSchemaParam.offline_pages,
+                        result_model_path=fetchSchemaParam.result_model_path,
+                    )
                 )
-            )
-            results.append(res)
-            first = False
+                results.append(res)
+                first = False
+        finally:
+            if not site_cache_state:
+                self.site.disable_cache()  # restore original state
 
         # merge unique results and return
         merged_result = OSW.FetchSchemaResult(
@@ -594,9 +606,13 @@ def _fetch_schema(
         ----------
         fetchSchemaParam
             See FetchSchemaParam, by default None
+
+        Notes
+        -----
+        The page cache is enabled and restored by the calling fetch_schema(), not
+        here. This method is called once per schema title and recursively per $ref,
+        so a snapshot taken here would read the state a previous call has set.
         """
-        site_cache_state = self.site.get_cache_enabled()
-        self.site.enable_cache()
         if fetchSchemaParam is None:
             fetchSchemaParam = OSW._FetchSchemaParam()
         schema_title = fetchSchemaParam.schema_title
@@ -1056,8 +1072,6 @@ def _fetch_schema(
 
             if fetchSchemaParam.final:
                 importlib.reload(model)  # reload the updated module
-                if not site_cache_state:
-                    self.site.disable_cache()  # restore original state
 
         return OSW.FetchSchemaResult(
             fetched_schema_titles=fetchSchemaParam.fetched_schema_titles,

tests/test_fetch_schema_cache.py

@@ -0,0 +1,136 @@
+"""Unit tests for the page cache handling of OSW.fetch_schema().
+
+Regression guard for #176: the cache state was taken and restored once per
+schema title inside _fetch_schema(). With two or more titles the second
+snapshot already read the state the first call had set, so the restore never
+disabled the cache again. An early return or an exception skipped the restore
+as well. fetch_schema() must now take the state once and restore it in every
+case.
+"""
+
+import threading
+
+import pytest
+
+from osw.core import OSW
+from osw.wtsite import WtSite
+
+
+def _make_fake_wtsite(cache_enabled: bool) -> WtSite:
+    """A WtSite that performs no network calls, with a known cache state."""
+    ws = WtSite.__new__(WtSite)
+    ws._session_lock = threading.RLock()
+    ws._page_cache = {}
+    ws._cache_enabled = cache_enabled
+    return ws
+
+
+def _make_osw(cache_enabled: bool) -> OSW:
+    """An OSW bound to that WtSite, bypassing __init__ and validation."""
+    return OSW.construct(site=_make_fake_wtsite(cache_enabled))
+
+
+def _stub_fetch_schema(
+    monkeypatch, seen_states: list, fail_on: str = None, leaks: bool = False
+):
+    """Replace the per-title worker. Records the cache state it is called with.
+
+    With leaks=True the worker enables the cache and never restores it, which is
+    what the previous _fetch_schema() did for every title but the last one.
+    """
+
+    def stub(self, fetchSchemaParam=None):
+        seen_states.append(self.site.get_cache_enabled())
+        if leaks:
+            self.site.enable_cache()
+        if fail_on is not None and fetchSchemaParam.schema_title == fail_on:
+            raise RuntimeError("fetching the schema failed")
+        return OSW.FetchSchemaResult(
+            fetched_schema_titles=[fetchSchemaParam.schema_title]
+        )
+
+    monkeypatch.setattr(OSW, "_fetch_schema", stub)
+
+
+def test_cache_is_disabled_again_after_several_titles(monkeypatch):
+    osw_obj = _make_osw(cache_enabled=False)
+    seen_states = []
+    _stub_fetch_schema(monkeypatch, seen_states)
+
+    osw_obj.fetch_schema(
+        OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
+    )
+
+    assert seen_states == [True, True]
+    assert osw_obj.site.get_cache_enabled() is False
+
+
+def test_cache_is_disabled_again_after_a_single_title(monkeypatch):
+    osw_obj = _make_osw(cache_enabled=False)
+    seen_states = []
+    _stub_fetch_schema(monkeypatch, seen_states)
+
+    osw_obj.fetch_schema(OSW.FetchSchemaParam(schema_title="Category:Item"))
+
+    assert seen_states == [True]
+    assert osw_obj.site.get_cache_enabled() is False
+
+
+def test_cache_stays_enabled_if_the_caller_had_it_enabled(monkeypatch):
+    osw_obj = _make_osw(cache_enabled=True)
+    seen_states = []
+    _stub_fetch_schema(monkeypatch, seen_states)
+
+    osw_obj.fetch_schema(
+        OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
+    )
+
+    assert seen_states == [True, True]
+    assert osw_obj.site.get_cache_enabled() is True
+
+
+def test_a_worker_that_leaves_the_cache_enabled_does_not_leak(monkeypatch):
+    """The reported defect: the per-title worker enabled the cache and kept it."""
+    osw_obj = _make_osw(cache_enabled=False)
+    seen_states = []
+    _stub_fetch_schema(monkeypatch, seen_states, leaks=True)
+
+    osw_obj.fetch_schema(
+        OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
+    )
+
+    assert osw_obj.site.get_cache_enabled() is False
+
+
+def test_cache_is_restored_when_a_title_raises(monkeypatch):
+    osw_obj = _make_osw(cache_enabled=False)
+    seen_states = []
+    _stub_fetch_schema(monkeypatch, seen_states, fail_on="Category:Entity", leaks=True)
+
+    with pytest.raises(RuntimeError):
+        osw_obj.fetch_schema(
+            OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
+        )
+
+    assert osw_obj.site.get_cache_enabled() is False
+
+
+def test_cache_is_restored_when_the_last_title_returns_early(monkeypatch):
+    """A missing schema page returns before the end of _fetch_schema()."""
+    osw_obj = _make_osw(cache_enabled=False)
+
+    def stub(self, fetchSchemaParam=None):
+        # mirrors the early return for a page that does not exist, which happens
+        # after the previous _fetch_schema() had enabled the cache
+        self.site.enable_cache()
+        return OSW.FetchSchemaResult(
+            error_messages=[f"Page {fetchSchemaParam.schema_title} does not exist"]
+        )
+
+    monkeypatch.setattr(OSW, "_fetch_schema", stub)
+
+    osw_obj.fetch_schema(
+        OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Missing"])
+    )
+
+    assert osw_obj.site.get_cache_enabled() is False