Skip to content

Commit 3a7b6eb

Browse files
committed
perf(file): cache the wiki's allowed file extensions for 24 h
- WtSite.get_allowed_file_extensions() caches the siteinfo lookup for ALLOWED_FILE_EXTENSIONS_TTL, so a failed upload no longer re-queries - clear_allowed_file_extensions_cache() resets it explicitly; the page cache and clear_cache() stay independent - a failed or malformed lookup is never cached and never propagates - the rejected-extension ValueError now names the fetch time, the cache lifetime and the reset call - reraise_upload_error takes the WtSite instead of the raw mwclient site
1 parent 186e9e3 commit 3a7b6eb

4 files changed

Lines changed: 294 additions & 35 deletions

File tree

‎src/osw/controller/file/wiki.py‎

Lines changed: 16 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,21 @@
1111
from osw.wtsite import WtSite
1212

1313

14-
def get_allowed_file_extensions(mw_site) -> Optional[List[str]]:
15-
"""Queries the file extensions the wiki accepts, None if unavailable"""
16-
try:
17-
result = mw_site.api(
18-
"query", meta="siteinfo", siprop="fileextensions", formatversion=2
19-
)
20-
except Exception:
21-
# only used to enrich an error message, never worth failing over
22-
return None
23-
extensions = result.get("query", {}).get("fileextensions", [])
24-
return [entry["ext"] for entry in extensions if "ext" in entry]
14+
def format_allowed_extensions(allowed: WtSite.AllowedFileExtensionsResult) -> str:
15+
"""Renders the allowed-extensions hint for a rejected-upload error message"""
16+
if not allowed.extensions or allowed.fetched_at is None:
17+
return ""
18+
ttl_hours = int(WtSite.ALLOWED_FILE_EXTENSIONS_TTL.total_seconds() // 3600)
19+
return (
20+
f" Extensions allowed there, read {allowed.fetched_at:%Y-%m-%d %H:%M:%S} and "
21+
f"cached for {ttl_hours} h: {', '.join(sorted(allowed.extensions))}. "
22+
"Call osw.site.clear_allowed_file_extensions_cache() to re-read the list "
23+
"if the wiki configuration changed since."
24+
)
2525

2626

2727
def reraise_upload_error(
28-
error: mwclient.errors.APIError, mw_site, title: str, suffix: Optional[str]
28+
error: mwclient.errors.APIError, site: WtSite, title: str, suffix: Optional[str]
2929
) -> None:
3030
"""Turns a rejected file extension into a readable error, re-raises the rest
3131
@@ -34,15 +34,11 @@ def reraise_upload_error(
3434
"""
3535
if "filetype" not in str(getattr(error, "code", "")):
3636
raise error
37-
allowed = get_allowed_file_extensions(mw_site)
38-
hint = (
39-
f" Extensions allowed on this wiki: {', '.join(sorted(allowed))}."
40-
if allowed
41-
else ""
42-
)
37+
allowed = site.get_allowed_file_extensions()
38+
hint = format_allowed_extensions(allowed)
4339
raise ValueError(
4440
f"Upload of '{title}' was rejected because the file extension "
45-
f"'{suffix}' is not allowed on {mw_site.host}.{hint}"
41+
f"'{suffix}' is not allowed on {site.mw_site.host}.{hint}"
4642
) from error
4743

4844

@@ -216,7 +212,7 @@ def put(self, file: IO, **kwargs: Dict[str, Any]):
216212
ignore=True,
217213
)
218214
except mwclient.errors.APIError as e:
219-
reraise_upload_error(e, self.osw.mw_site, self.title, self.suffix)
215+
reraise_upload_error(e, self.osw.site, self.title, self.suffix)
220216
assert_upload_success(result, self.title, self.osw.mw_site.host)
221217
self.osw.store_entity(
222218
OSW.StoreEntityParam(

‎src/osw/wtsite.py‎

Lines changed: 70 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
import warnings
1111
import xml.etree.ElementTree as et
1212
from copy import deepcopy
13-
from datetime import datetime
13+
from datetime import datetime, timedelta
1414
from io import StringIO
1515
from pathlib import Path
1616
from pprint import pprint
@@ -79,6 +79,9 @@ class WtSiteLegacyConfig(OswBaseModel):
7979
class Config:
8080
arbitrary_types_allowed = True
8181

82+
ALLOWED_FILE_EXTENSIONS_TTL = timedelta(hours=24)
83+
"""how long a fetched list of accepted file extensions stays valid"""
84+
8285
def __init__(self, config: Union[WtSiteConfig, WtSiteLegacyConfig]):
8386
"""creates a new WtSite instance from a WtSiteConfig
8487
@@ -161,6 +164,10 @@ def __init__(self, config: Union[WtSiteConfig, WtSiteLegacyConfig]):
161164
self._page_cache = {}
162165
self._cache_enabled = False
163166

167+
# The file extensions the wiki accepts, read on demand and reused for
168+
# ALLOWED_FILE_EXTENSIONS_TTL
169+
self._allowed_file_extensions = None
170+
164171
def _get_session_lock(self) -> threading.RLock:
165172
"""Return the session lock, lazily creating it if absent.
166173
@@ -492,6 +499,68 @@ def clear_cache(self):
492499
del self._page_cache
493500
self._page_cache = {}
494501

502+
class AllowedFileExtensionsResult(OswBaseModel):
503+
"""The file extensions a wiki accepts and when that list was read"""
504+
505+
extensions: Optional[List[str]]
506+
"""the accepted extensions, None if the lookup failed"""
507+
fetched_at: Optional[datetime]
508+
"""when the list was read from the wiki, None if the lookup failed"""
509+
510+
def get_allowed_file_extensions(
511+
self, refresh: bool = False
512+
) -> AllowedFileExtensionsResult:
513+
"""Returns the file extensions the wiki accepts, from cache if possible
514+
515+
The list only changes when the wiki configuration changes, so it is
516+
cached for ALLOWED_FILE_EXTENSIONS_TTL instead of being read on every
517+
call. A failed lookup is never cached, so the next call retries; a
518+
failed refresh leaves any previously cached list in place.
519+
520+
Parameters
521+
----------
522+
refresh:
523+
if True, bypasses the cache and re-reads the list from the wiki
524+
525+
Returns
526+
-------
527+
an AllowedFileExtensionsResult with the accepted extensions and
528+
when they were read, or with both fields None if the lookup failed
529+
"""
530+
cached = getattr(self, "_allowed_file_extensions", None)
531+
if (
532+
cached is not None
533+
and not refresh
534+
and cached.fetched_at is not None
535+
and datetime.now() - cached.fetched_at < self.ALLOWED_FILE_EXTENSIONS_TTL
536+
):
537+
return cached
538+
try:
539+
result = self.mw_site.api(
540+
"query", meta="siteinfo", siprop="fileextensions", formatversion=2
541+
)
542+
extensions = [
543+
entry["ext"]
544+
for entry in result.get("query", {}).get("fileextensions", [])
545+
if "ext" in entry
546+
]
547+
except Exception:
548+
# only used to enrich an error message, never worth failing over
549+
return WtSite.AllowedFileExtensionsResult(extensions=None, fetched_at=None)
550+
self._allowed_file_extensions = WtSite.AllowedFileExtensionsResult(
551+
extensions=extensions, fetched_at=datetime.now()
552+
)
553+
return self._allowed_file_extensions
554+
555+
def clear_allowed_file_extensions_cache(self):
556+
"""Clears the cached list of file extensions the wiki accepts
557+
558+
The next get_allowed_file_extensions() call reads the list from the
559+
wiki again. Use this when the wiki's upload configuration changed
560+
within ALLOWED_FILE_EXTENSIONS_TTL.
561+
"""
562+
self._allowed_file_extensions = None
563+
495564
def _clear_cookies(self):
496565
# see https://github.com/mwclient/mwclient/issues/221
497566
# Iterate a snapshot (list(...)) so mutating the jar mid-loop is safe, and

‎tests/test_wiki_file_upload_errors.py‎

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,25 +13,38 @@
1313

1414
from osw.controller.file.wiki import (
1515
assert_upload_success,
16-
get_allowed_file_extensions,
16+
format_allowed_extensions,
1717
reraise_upload_error,
1818
store_params_for_upload,
1919
)
20+
from osw.wtsite import WtSite
2021

2122

22-
class _FakeSite:
23+
class _FakeMwSite:
24+
"""Stands in for the mwclient.Site behind WtSite.mw_site."""
25+
2326
host = "wiki.example.org"
2427

2528
def __init__(self, extensions=None, fail=False):
26-
self._extensions = extensions
29+
self._extensions = extensions or []
2730
self._fail = fail
31+
self.api_calls = 0
2832

2933
def api(self, *args, **kwargs):
34+
self.api_calls += 1
3035
if self._fail:
3136
raise RuntimeError("siteinfo unavailable")
3237
return {"query": {"fileextensions": [{"ext": e} for e in self._extensions]}}
3338

3439

40+
def _fake_site(extensions=None, fail=False):
41+
"""Builds a minimal WtSite carrying a fake mwclient site, bypassing __init__."""
42+
ws = WtSite.__new__(WtSite)
43+
ws._site = _FakeMwSite(extensions=extensions, fail=fail)
44+
ws._allowed_file_extensions = None
45+
return ws
46+
47+
3548
def _api_error(code):
3649
return mwclient.errors.APIError(code, "info", {})
3750

@@ -40,7 +53,7 @@ def _api_error(code):
4053
"code", ["filetype-banned", "filetype-banned-type", "filetype-badtype"]
4154
)
4255
def test_rejected_extension_names_the_extension(code):
43-
site = _FakeSite(extensions=["png", "pdf"])
56+
site = _fake_site(extensions=["png", "pdf"])
4457

4558
with pytest.raises(ValueError) as exc_info:
4659
reraise_upload_error(_api_error(code), site, "OSW123.exe", ".exe")
@@ -53,7 +66,7 @@ def test_rejected_extension_names_the_extension(code):
5366

5467

5568
def test_original_error_is_chained():
56-
site = _FakeSite(extensions=["png"])
69+
site = _fake_site(extensions=["png"])
5770
original = _api_error("filetype-banned")
5871

5972
with pytest.raises(ValueError) as exc_info:
@@ -63,7 +76,7 @@ def test_original_error_is_chained():
6376

6477

6578
def test_unrelated_api_error_is_reraised_unchanged():
66-
site = _FakeSite(extensions=["png"])
79+
site = _fake_site(extensions=["png"])
6780
original = _api_error("readapidenied")
6881

6982
with pytest.raises(mwclient.errors.APIError) as exc_info:
@@ -74,23 +87,50 @@ def test_unrelated_api_error_is_reraised_unchanged():
7487

7588
def test_message_omits_the_hint_when_siteinfo_fails():
7689
"""A failing siteinfo lookup must not mask the upload error."""
77-
site = _FakeSite(fail=True)
90+
site = _fake_site(fail=True)
91+
92+
with pytest.raises(ValueError) as exc_info:
93+
reraise_upload_error(_api_error("filetype-banned"), site, "a.exe", ".exe")
94+
95+
message = str(exc_info.value)
96+
assert "Extensions allowed" not in message
97+
assert "clear_allowed_file_extensions_cache" not in message
98+
assert ".exe" in message
99+
100+
101+
def test_message_names_the_fetch_time_and_the_reset_hint():
102+
site = _fake_site(extensions=["png", "pdf"])
78103

79104
with pytest.raises(ValueError) as exc_info:
80105
reraise_upload_error(_api_error("filetype-banned"), site, "a.exe", ".exe")
81106

82-
assert "allowed on this wiki" not in str(exc_info.value)
83-
assert ".exe" in str(exc_info.value)
107+
message = str(exc_info.value)
108+
fetched_at = site.get_allowed_file_extensions().fetched_at
109+
assert f"{fetched_at:%Y-%m-%d %H:%M:%S}" in message
110+
assert "osw.site.clear_allowed_file_extensions_cache()" in message
111+
112+
113+
def test_a_second_rejected_upload_does_not_query_the_wiki_again():
114+
site = _fake_site(extensions=["png", "pdf"])
115+
116+
with pytest.raises(ValueError):
117+
reraise_upload_error(_api_error("filetype-banned"), site, "a.exe", ".exe")
118+
with pytest.raises(ValueError):
119+
reraise_upload_error(_api_error("filetype-banned"), site, "b.exe", ".exe")
120+
121+
assert site.mw_site.api_calls == 1
122+
84123

124+
def test_format_allowed_extensions_is_empty_when_extensions_is_none():
125+
allowed = WtSite.AllowedFileExtensionsResult(extensions=None, fetched_at=None)
85126

86-
def test_get_allowed_file_extensions_returns_none_on_failure():
87-
assert get_allowed_file_extensions(_FakeSite(fail=True)) is None
127+
assert format_allowed_extensions(allowed) == ""
88128

89129

90-
def test_get_allowed_file_extensions_reads_siteinfo():
91-
site = _FakeSite(extensions=["png", "jpg"])
130+
def test_format_allowed_extensions_is_empty_when_extensions_is_empty():
131+
allowed = WtSite.AllowedFileExtensionsResult(extensions=[], fetched_at=None)
92132

93-
assert get_allowed_file_extensions(site) == ["png", "jpg"]
133+
assert format_allowed_extensions(allowed) == ""
94134

95135

96136
def test_successful_upload_passes():

0 commit comments

Comments
 (0)