Skip to content

Commit 477f4ca

Browse files
authored
Merge pull request #180 from OpenSemanticLab/fix/176-fetch-schema-cache
fix(core): restore the page cache state once per fetch_schema call
2 parents 12ecbdd + 727032f commit 477f4ca

2 files changed

Lines changed: 171 additions & 21 deletions

File tree

‎src/osw/core.py‎

Lines changed: 35 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -917,24 +917,36 @@ def fetch_schema(
917917
first = True
918918
last = False
919919
results = []
920-
for schema_title in fetchSchemaParam.schema_title:
921-
last = schema_title == fetchSchemaParam.schema_title[-1]
922-
mode = fetchSchemaParam.mode
923-
if not first: # 'replace' makes only sense for the first schema
924-
mode = "append"
925-
res = self._fetch_schema(
926-
OSW._FetchSchemaParam(
927-
schema_title=schema_title,
928-
mode=mode,
929-
final=last,
930-
generate_annotations=fetchSchemaParam.generate_annotations,
931-
generator_options=fetchSchemaParam.generator_options,
932-
offline_pages=fetchSchemaParam.offline_pages,
933-
result_model_path=fetchSchemaParam.result_model_path,
920+
# the page cache is enabled once for the whole operation, because the same
921+
# schema pages are read repeatedly while $refs are resolved. The state is
922+
# taken and restored here and not per schema title: _fetch_schema would
923+
# snapshot the state its own predecessor has already changed. The restore
924+
# runs in a finally block so that an early return or an exception in
925+
# _fetch_schema cannot leave the cache enabled for the rest of the process.
926+
site_cache_state = self.site.get_cache_enabled()
927+
self.site.enable_cache()
928+
try:
929+
for schema_title in fetchSchemaParam.schema_title:
930+
last = schema_title == fetchSchemaParam.schema_title[-1]
931+
mode = fetchSchemaParam.mode
932+
if not first: # 'replace' makes only sense for the first schema
933+
mode = "append"
934+
res = self._fetch_schema(
935+
OSW._FetchSchemaParam(
936+
schema_title=schema_title,
937+
mode=mode,
938+
final=last,
939+
generate_annotations=fetchSchemaParam.generate_annotations,
940+
generator_options=fetchSchemaParam.generator_options,
941+
offline_pages=fetchSchemaParam.offline_pages,
942+
result_model_path=fetchSchemaParam.result_model_path,
943+
)
934944
)
935-
)
936-
results.append(res)
937-
first = False
945+
results.append(res)
946+
first = False
947+
finally:
948+
if not site_cache_state:
949+
self.site.disable_cache() # restore original state
938950

939951
# merge unique results and return
940952
merged_result = OSW.FetchSchemaResult(
@@ -1015,9 +1027,13 @@ def _fetch_schema(
10151027
----------
10161028
fetchSchemaParam
10171029
See FetchSchemaParam, by default None
1030+
1031+
Notes
1032+
-----
1033+
The page cache is enabled and restored by the calling fetch_schema(), not
1034+
here. This method is called once per schema title and recursively per $ref,
1035+
so a snapshot taken here would read the state a previous call has set.
10181036
"""
1019-
site_cache_state = self.site.get_cache_enabled()
1020-
self.site.enable_cache()
10211037
if fetchSchemaParam is None:
10221038
fetchSchemaParam = OSW._FetchSchemaParam()
10231039
schema_title = fetchSchemaParam.schema_title
@@ -1493,8 +1509,6 @@ def _fetch_schema(
14931509
# reload the updated module, restoring the previous content if
14941510
# the generated model turns out not to be importable
14951511
reload_module_or_restore(model, result_model_path, previous_content)
1496-
if not site_cache_state:
1497-
self.site.disable_cache() # restore original state
14981512

14991513
return OSW.FetchSchemaResult(
15001514
fetched_schema_titles=fetchSchemaParam.fetched_schema_titles,

‎tests/test_fetch_schema_cache.py‎

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
"""Unit tests for the page cache handling of OSW.fetch_schema().
2+
3+
Regression guard for #176: the cache state was taken and restored once per
4+
schema title inside _fetch_schema(). With two or more titles the second
5+
snapshot already read the state the first call had set, so the restore never
6+
disabled the cache again. An early return or an exception skipped the restore
7+
as well. fetch_schema() must now take the state once and restore it in every
8+
case.
9+
"""
10+
11+
import threading
12+
13+
import pytest
14+
15+
from osw.core import OSW
16+
from osw.wtsite import WtSite
17+
18+
19+
def _make_fake_wtsite(cache_enabled: bool) -> WtSite:
20+
"""A WtSite that performs no network calls, with a known cache state."""
21+
ws = WtSite.__new__(WtSite)
22+
ws._session_lock = threading.RLock()
23+
ws._page_cache = {}
24+
ws._cache_enabled = cache_enabled
25+
return ws
26+
27+
28+
def _make_osw(cache_enabled: bool) -> OSW:
29+
"""An OSW bound to that WtSite, bypassing __init__ and validation."""
30+
return OSW.construct(site=_make_fake_wtsite(cache_enabled))
31+
32+
33+
def _stub_fetch_schema(
34+
monkeypatch, seen_states: list, fail_on: str = None, leaks: bool = False
35+
):
36+
"""Replace the per-title worker. Records the cache state it is called with.
37+
38+
With leaks=True the worker enables the cache and never restores it, which is
39+
what the previous _fetch_schema() did for every title but the last one.
40+
"""
41+
42+
def stub(self, fetchSchemaParam=None):
43+
seen_states.append(self.site.get_cache_enabled())
44+
if leaks:
45+
self.site.enable_cache()
46+
if fail_on is not None and fetchSchemaParam.schema_title == fail_on:
47+
raise RuntimeError("fetching the schema failed")
48+
return OSW.FetchSchemaResult(
49+
fetched_schema_titles=[fetchSchemaParam.schema_title]
50+
)
51+
52+
monkeypatch.setattr(OSW, "_fetch_schema", stub)
53+
54+
55+
def test_cache_is_disabled_again_after_several_titles(monkeypatch):
56+
osw_obj = _make_osw(cache_enabled=False)
57+
seen_states = []
58+
_stub_fetch_schema(monkeypatch, seen_states)
59+
60+
osw_obj.fetch_schema(
61+
OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
62+
)
63+
64+
assert seen_states == [True, True]
65+
assert osw_obj.site.get_cache_enabled() is False
66+
67+
68+
def test_cache_is_disabled_again_after_a_single_title(monkeypatch):
69+
osw_obj = _make_osw(cache_enabled=False)
70+
seen_states = []
71+
_stub_fetch_schema(monkeypatch, seen_states)
72+
73+
osw_obj.fetch_schema(OSW.FetchSchemaParam(schema_title="Category:Item"))
74+
75+
assert seen_states == [True]
76+
assert osw_obj.site.get_cache_enabled() is False
77+
78+
79+
def test_cache_stays_enabled_if_the_caller_had_it_enabled(monkeypatch):
80+
osw_obj = _make_osw(cache_enabled=True)
81+
seen_states = []
82+
_stub_fetch_schema(monkeypatch, seen_states)
83+
84+
osw_obj.fetch_schema(
85+
OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
86+
)
87+
88+
assert seen_states == [True, True]
89+
assert osw_obj.site.get_cache_enabled() is True
90+
91+
92+
def test_a_worker_that_leaves_the_cache_enabled_does_not_leak(monkeypatch):
93+
"""The reported defect: the per-title worker enabled the cache and kept it."""
94+
osw_obj = _make_osw(cache_enabled=False)
95+
seen_states = []
96+
_stub_fetch_schema(monkeypatch, seen_states, leaks=True)
97+
98+
osw_obj.fetch_schema(
99+
OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
100+
)
101+
102+
assert osw_obj.site.get_cache_enabled() is False
103+
104+
105+
def test_cache_is_restored_when_a_title_raises(monkeypatch):
106+
osw_obj = _make_osw(cache_enabled=False)
107+
seen_states = []
108+
_stub_fetch_schema(monkeypatch, seen_states, fail_on="Category:Entity", leaks=True)
109+
110+
with pytest.raises(RuntimeError):
111+
osw_obj.fetch_schema(
112+
OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Entity"])
113+
)
114+
115+
assert osw_obj.site.get_cache_enabled() is False
116+
117+
118+
def test_cache_is_restored_when_the_last_title_returns_early(monkeypatch):
119+
"""A missing schema page returns before the end of _fetch_schema()."""
120+
osw_obj = _make_osw(cache_enabled=False)
121+
122+
def stub(self, fetchSchemaParam=None):
123+
# mirrors the early return for a page that does not exist, which happens
124+
# after the previous _fetch_schema() had enabled the cache
125+
self.site.enable_cache()
126+
return OSW.FetchSchemaResult(
127+
error_messages=[f"Page {fetchSchemaParam.schema_title} does not exist"]
128+
)
129+
130+
monkeypatch.setattr(OSW, "_fetch_schema", stub)
131+
132+
osw_obj.fetch_schema(
133+
OSW.FetchSchemaParam(schema_title=["Category:Item", "Category:Missing"])
134+
)
135+
136+
assert osw_obj.site.get_cache_enabled() is False

0 commit comments

Comments
 (0)