Refactor the cachedownloader to bound/unbound

Separate the existing singular downloader into two distinct objects:
the bound and unbound variants. An unbound downloader implements the
core logic, almost to completion. A bound downloader *contains* an
unbound one and adds a known file target (remote and local names to
use).

The two are tied together via a single method:

    CacheDownloader.bind(URI, name) -> BoundCacheDownloader

The result allows for a CacheDownloader to be built and then bound
multiple times.

Author: sirosen

src/check_jsonschema/cachedownloader.py

@@ -11,103 +11,102 @@
 
 import requests
 
+# this will let us do any other caching we might need in the future in the same
+# cache dir (adjacent to "downloads")
+_CACHEDIR_NAME = os.path.join("check_jsonschema", "downloads")
+
+_LASTMOD_FMT = "%a, %d %b %Y %H:%M:%S %Z"
+
+
+def _get_default_cache_dir() -> str | None:
+    sysname = platform.system()
+
+    # on windows, try to get the appdata env var
+    # this *could* result in cache_dir=None, which is fine, just skip caching in
+    # that case
+    if sysname == "Windows":
+        cache_dir = os.getenv("LOCALAPPDATA", os.getenv("APPDATA"))
+    # macOS -> app support dir
+    elif sysname == "Darwin":
+        cache_dir = os.path.expanduser("~/Library/Caches")
+    # default for unknown platforms, namely linux behavior
+    # use XDG env var and default to ~/.cache/
+    else:
+        cache_dir = os.getenv("XDG_CACHE_HOME", os.path.expanduser("~/.cache"))
+
+    if cache_dir:
+        cache_dir = os.path.join(cache_dir, _CACHEDIR_NAME)
+
+    return cache_dir
+
+
+def _lastmod_from_response(response: requests.Response) -> float:
+    try:
+        return time.mktime(
+            time.strptime(response.headers["last-modified"], _LASTMOD_FMT)
+        )
+    # OverflowError: time outside of platform-specific bounds
+    # ValueError: malformed/unparseable
+    # LookupError: no such header
+    except (OverflowError, ValueError, LookupError):
+        return 0.0
+
+
+def _get_request(
+    file_url: str, *, response_ok: t.Callable[[requests.Response], bool]
+) -> requests.Response:
+    try:
+        r: requests.Response | None = None
+        for _attempt in range(3):
+            r = requests.get(file_url, stream=True)
+            if r.ok and response_ok(r):
+                return r
+        assert r is not None
+        raise FailedDownloadError(
+            f"got response with status={r.status_code}, retries exhausted"
+        )
+    except requests.RequestException as e:
+        raise FailedDownloadError("encountered error during download") from e
+
+
+def _atomic_write(dest: str, content: bytes) -> None:
+    # download to a temp file and then move to the dest
+    # this makes the download safe if run in parallel (parallel runs
+    # won't create a new empty file for writing and cause failures)
+    fp = tempfile.NamedTemporaryFile(mode="wb", delete=False)
+    fp.write(content)
+    fp.close()
+    shutil.copy(fp.name, dest)
+    os.remove(fp.name)
+
+
+def _cache_hit(cachefile: str, response: requests.Response) -> bool:
+    # no file? miss
+    if not os.path.exists(cachefile):
+        return False
+
+    # compare mtime on any cached file against the remote last-modified time
+    # it is considered a hit if the local file is at least as new as the remote file
+    local_mtime = os.path.getmtime(cachefile)
+    remote_mtime = _lastmod_from_response(response)
+    return local_mtime >= remote_mtime
+
 
 class FailedDownloadError(Exception):
     pass
 
 
 class CacheDownloader:
-    _LASTMOD_FMT = "%a, %d %b %Y %H:%M:%S %Z"
-
-    # changed in v0.5.0
-    # original cache dir was "jsonschema_validate"
-    # this will let us do any other caching we might need in the future in the same
-    # cache dir (adjacent to "downloads")
-    _CACHEDIR_NAME = os.path.join("check_jsonschema", "downloads")
-
     def __init__(
         self,
-        file_url: str,
-        filename: str | None = None,
         cache_dir: str | None = None,
         disable_cache: bool = False,
         validation_callback: t.Callable[[bytes], t.Any] | None = None,
     ):
-        self._file_url = file_url
-        self._filename = filename or file_url.split("/")[-1]
-        self._cache_dir = cache_dir or self._compute_default_cache_dir()
+        self._cache_dir = cache_dir or _get_default_cache_dir()
         self._disable_cache = disable_cache
         self._validation_callback = validation_callback
 
-    def _compute_default_cache_dir(self) -> str | None:
-        sysname = platform.system()
-
-        # on windows, try to get the appdata env var
-        # this *could* result in cache_dir=None, which is fine, just skip caching in
-        # that case
-        if sysname == "Windows":
-            cache_dir = os.getenv("LOCALAPPDATA", os.getenv("APPDATA"))
-        # macOS -> app support dir
-        elif sysname == "Darwin":
-            cache_dir = os.path.expanduser("~/Library/Caches")
-        # default for unknown platforms, namely linux behavior
-        # use XDG env var and default to ~/.cache/
-        else:
-            cache_dir = os.getenv("XDG_CACHE_HOME", os.path.expanduser("~/.cache"))
-
-        if cache_dir:
-            cache_dir = os.path.join(cache_dir, self._CACHEDIR_NAME)
-
-        return cache_dir
-
-    def _get_request(
-        self, *, response_ok: t.Callable[[requests.Response], bool]
-    ) -> requests.Response:
-        try:
-            r: requests.Response | None = None
-            for _attempt in range(3):
-                r = requests.get(self._file_url, stream=True)
-                if r.ok and response_ok(r):
-                    return r
-            assert r is not None
-            raise FailedDownloadError(
-                f"got response with status={r.status_code}, retries exhausted"
-            )
-        except requests.RequestException as e:
-            raise FailedDownloadError("encountered error during download") from e
-
-    def _lastmod_from_response(self, response: requests.Response) -> float:
-        try:
-            return time.mktime(
-                time.strptime(response.headers["last-modified"], self._LASTMOD_FMT)
-            )
-        # OverflowError: time outside of platform-specific bounds
-        # ValueError: malformed/unparseable
-        # LookupError: no such header
-        except (OverflowError, ValueError, LookupError):
-            return 0.0
-
-    def _cache_hit(self, cachefile: str, response: requests.Response) -> bool:
-        # no file? miss
-        if not os.path.exists(cachefile):
-            return False
-
-        # compare mtime on any cached file against the remote last-modified time
-        # it is considered a hit if the local file is at least as new as the remote file
-        local_mtime = os.path.getmtime(cachefile)
-        remote_mtime = self._lastmod_from_response(response)
-        return local_mtime >= remote_mtime
-
-    def _write(self, dest: str, response: requests.Response) -> None:
-        # download to a temp file and then move to the dest
-        # this makes the download safe if run in parallel (parallel runs
-        # won't create a new empty file for writing and cause failures)
-        fp = tempfile.NamedTemporaryFile(mode="wb", delete=False)
-        fp.write(response.content)
-        fp.close()
-        shutil.copy(fp.name, dest)
-        os.remove(fp.name)
-
     def _validate(self, response: requests.Response) -> bool:
         if not self._validation_callback:
             return True
@@ -118,32 +117,52 @@ def _validate(self, response: requests.Response) -> bool:
         except ValueError:
             return False
 
-    def _download(self) -> str:
-        assert self._cache_dir
+    def _download(self, file_url: str, filename: str) -> str:
+        assert self._cache_dir is not None
         os.makedirs(self._cache_dir, exist_ok=True)
-        dest = os.path.join(self._cache_dir, self._filename)
+        dest = os.path.join(self._cache_dir, filename)
 
         def check_response_for_download(r: requests.Response) -> bool:
             # if the response indicates a cache hit, treat it as valid
             # this ensures that we short-circuit any further evaluation immediately on
             # a hit
-            if self._cache_hit(dest, r):
+            if _cache_hit(dest, r):
                 return True
             # we now know it's not a hit, so validate the content (forces download)
             return self._validate(r)
 
-        response = self._get_request(response_ok=check_response_for_download)
+        response = _get_request(file_url, response_ok=check_response_for_download)
         # check to see if we have a file which matches the connection
         # only download if we do not (cache miss, vs hit)
-        if not self._cache_hit(dest, response):
-            self._write(dest, response)
+        if not _cache_hit(dest, response):
+            _atomic_write(dest, response.content)
 
         return dest
 
     @contextlib.contextmanager
-    def open(self) -> t.Iterator[t.IO[bytes]]:
+    def open(self, file_url: str, filename: str) -> t.Iterator[t.IO[bytes]]:
         if (not self._cache_dir) or self._disable_cache:
-            yield io.BytesIO(self._get_request(response_ok=self._validate).content)
+            yield io.BytesIO(_get_request(file_url, response_ok=self._validate).content)
         else:
-            with open(self._download(), "rb") as fp:
+            with open(self._download(file_url, filename), "rb") as fp:
                 yield fp
+
+    def bind(self, file_url: str, filename: str | None = None) -> BoundCacheDownloader:
+        return BoundCacheDownloader(file_url, filename, self)
+
+
+class BoundCacheDownloader:
+    def __init__(
+        self,
+        file_url: str,
+        filename: str | None,
+        downloader: CacheDownloader,
+    ):
+        self._file_url = file_url
+        self._filename = filename or file_url.split("/")[-1]
+        self._downloader = downloader
+
+    @contextlib.contextmanager
+    def open(self) -> t.Iterator[t.IO[bytes]]:
+        with self._downloader.open(self._file_url, self._filename) as fp:
+            yield fp

src/check_jsonschema/schema_loader/readers.py

@@ -79,11 +79,9 @@ def __init__(
         self.url = url
         self.parsers = ParserSet()
         self.downloader = CacheDownloader(
-            url,
-            cache_filename,
             disable_cache=disable_cache,
             validation_callback=self._parse,
-        )
+        ).bind(url, cache_filename)
         self._parsed_schema: dict | _UnsetType = _UNSET
 
     def _parse(self, schema_bytes: bytes) -> t.Any:

tests/acceptance/test_remote_ref_resolution.py

@@ -3,8 +3,6 @@
 import pytest
 import responses
 
-from check_jsonschema import cachedownloader
-
 CASES = {
     "case1": {
         "main_schema": {
@@ -39,13 +37,12 @@
 
 @pytest.fixture(autouse=True)
 def _mock_schema_cache_dir(monkeypatch, tmp_path):
-    def _fake_compute_default_cache_dir(self):
+    def _fake_default_cache_dir():
         return str(tmp_path)
 
     monkeypatch.setattr(
-        cachedownloader.CacheDownloader,
-        "_compute_default_cache_dir",
-        _fake_compute_default_cache_dir,
+        "check_jsonschema.cachedownloader._get_default_cache_dir",
+        _fake_default_cache_dir,
     )
 
 

tests/acceptance/test_special_filetypes.py

@@ -6,8 +6,6 @@
 import pytest
 import responses
 
-from check_jsonschema import cachedownloader
-
 
 @pytest.mark.skipif(
     platform.system() != "Linux", reason="test requires /proc/self/ mechanism"
@@ -87,13 +85,12 @@ def test_remote_schema_requiring_retry(run_line, check_passes, tmp_path, monkeyp
     fires in order to parse
     """
 
-    def _fake_compute_default_cache_dir(self):
+    def _fake_default_cache_dir():
         return str(tmp_path)
 
     monkeypatch.setattr(
-        cachedownloader.CacheDownloader,
-        "_compute_default_cache_dir",
-        _fake_compute_default_cache_dir,
+        "check_jsonschema.cachedownloader._get_default_cache_dir",
+        _fake_default_cache_dir,
     )
 
     schema_loc = "https://example.com/schema1.json"