Merge pull request #41 from cachedjdk/restore-modularity

Restore separation of concerns between internal modules

Author: marktsuchida

docs/development.md

@@ -53,6 +53,23 @@ New notebook pages can be added by first creating the notebook (`.ipynb`) in
 Jupyter Lab, then running `jupytext mypage.ipynb --to myst`. Delete the
 `.ipynb` file so that the MyST (`.md`) file is the single source of truth.
 
+## Modularity
+
+We try to maintain good separation of concerns between modules, each of which
+should have a single area of responsibility. The division will undoubtedly
+evolve over time. See each module's docstring for their intended scope (and
+keep it up to date!). Think hard before introducing new dependencies between
+modules.
+
+All internal modules (which currently are all modules except for `__init__.py`)
+begin with an underscore. The public API is defined by `_api` and `_exceptions`
+and exposed by `__init__.py`. The command-line interface, defined in
+`__main__.py`, uses the public API only.
+
+All module-internal names are prefixed with an underscore. Such names should
+not be used from another module. Modules must avoid reaching into each other's
+internals.
+
 (versioning-scheme)=
 
 ## Versioning

src/cjdk/__main__.py

@@ -9,8 +9,7 @@
 
 import click
 
-from . import __version__, _api
-from ._exceptions import CjdkError
+from . import CjdkError, __version__, _api
 
 __all__ = [
     "main",

src/cjdk/_api.py

@@ -1,6 +1,14 @@
 # This file is part of cjdk.
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
+
+"""
+Public API surface.
+
+Exposes all user-facing functions (which are re-exported by __init__.py).
+Coordinates calls to other modules.
+"""
+
 from __future__ import annotations
 
 import hashlib
@@ -9,7 +17,7 @@
 from contextlib import contextmanager
 from typing import TYPE_CHECKING
 
-from . import _cache, _conf, _index, _install, _jdk
+from . import _conf, _install, _jdk
 from ._exceptions import (
     CjdkError,
     ConfigError,
@@ -64,7 +72,8 @@ def list_vendors(**kwargs: Unpack[ConfigKwargs]) -> list[str]:
     InstallError
         If fetching the index fails.
     """
-    return sorted(_get_vendors(**kwargs))
+    conf = _conf.configure(**kwargs)
+    return sorted(_jdk.available_vendors(conf))
 
 
 def list_jdks(  # type: ignore [misc]  # overlap with kwargs
@@ -113,9 +122,27 @@ def list_jdks(  # type: ignore [misc]  # overlap with kwargs
     InstallError
         If fetching the index fails.
     """
-    return _get_jdks(
-        vendor=vendor, version=version, cached_only=cached_only, **kwargs
-    )
+    jdk = kwargs.pop("jdk", None)
+    if jdk:
+        parsed_vendor, parsed_version = _conf.parse_vendor_version(jdk)
+        vendor = vendor or parsed_vendor or None
+        version = version or parsed_version or None
+
+    if vendor is None:
+        conf = _conf.configure(**kwargs)
+        return [
+            jdk
+            for v in sorted(_jdk.available_vendors(conf))
+            for jdk in list_jdks(
+                vendor=v,
+                version=version,
+                cached_only=cached_only,
+                **kwargs,
+            )
+        ]
+
+    conf = _conf.configure(vendor=vendor, version=version, **kwargs)
+    return _jdk.matching_jdks(conf, cached_only=cached_only)
 
 
 def clear_cache(**kwargs: Unpack[ConfigKwargs]) -> Path:
@@ -425,84 +452,6 @@ def cache_package(
         raise ConfigError(str(e)) from e
 
 
-def _get_vendors(**kwargs: Unpack[ConfigKwargs]) -> set[str]:
-    conf = _conf.configure(**kwargs)
-    index = _index.jdk_index(conf)
-    return {
-        vendor.replace("jdk@", "")
-        for osys in index
-        for arch in index[osys]
-        for vendor in index[osys][arch]
-    }
-
-
-def _get_jdks(
-    *,
-    vendor: str | None = None,
-    version: str | None = None,
-    cached_only: bool = True,
-    **kwargs: Unpack[ConfigKwargs],
-) -> list[str]:
-    jdk = kwargs.pop("jdk", None)
-    if jdk:
-        parsed_vendor, parsed_version = _conf.parse_vendor_version(jdk)
-        vendor = vendor or parsed_vendor or None
-        version = version or parsed_version or None
-
-    # Handle "all vendors" before creating Configuration.
-    if vendor is None:
-        return [
-            jdk
-            for v in sorted(_get_vendors())
-            for jdk in _get_jdks(
-                vendor=v,
-                version=version,
-                cached_only=cached_only,
-                **kwargs,
-            )
-        ]
-
-    conf = _conf.configure(vendor=vendor, version=version, **kwargs)
-    index = _index.jdk_index(conf)
-    jdks = _index.available_jdks(index, conf)
-    versions = _index._get_versions(jdks, conf)
-    matched = _index._match_versions(conf.vendor, versions, conf.version)
-
-    if cached_only:
-        # Filter matches by existing key directories.
-        def is_cached(v: str) -> bool:
-            url = _index.jdk_url(index, conf, v)
-            key = (_jdk._JDK_KEY_PREFIX, _cache._key_for_url(url))
-            keydir = _cache._key_directory(conf.cache_dir, key)
-            return keydir.exists()
-
-        matched = {k: v for k, v in matched.items() if is_cached(v)}
-
-    class VersionElement:
-        def __init__(self, value: int | str) -> None:
-            self.value = value
-
-        def __eq__(self, other: VersionElement) -> bool:  # type: ignore[override]
-            if isinstance(self.value, int) and isinstance(other.value, int):
-                return self.value == other.value
-            return str(self.value) == str(other.value)
-
-        def __lt__(self, other: VersionElement) -> bool:
-            if isinstance(self.value, int) and isinstance(other.value, int):
-                return self.value < other.value
-            return str(self.value) < str(other.value)
-
-    def version_key(
-        version_tuple: tuple[tuple[int | str, ...], str],
-    ) -> tuple[VersionElement, ...]:
-        return tuple(VersionElement(elem) for elem in version_tuple[0])
-
-    return [
-        f"{conf.vendor}:{v}"
-        for k, v in sorted(matched.items(), key=version_key)
-    ]
-
-
 def _make_hash_checker(
     hashes: dict[str, str | None],
 ) -> Callable[[Path], None]:

src/cjdk/_cache.py

@@ -1,6 +1,18 @@
 # This file is part of cjdk.
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
+
+"""
+Low-level caching primitives.
+
+Manages URL-to-cache-key mapping (via SHA-1 hashing of normalized URLs), atomic
+file/directory operations, TTL-based freshness checks, and inter-process
+coordination (waiting when another process is downloading the same file).
+
+No JDK-specific operations. All network/unarchive operations are
+dependency-injected.
+"""
+
 from __future__ import annotations
 
 import hashlib
@@ -19,10 +31,17 @@
 
 __all__ = [
     "atomic_file",
+    "is_cached",
     "permanent_directory",
 ]
 
 
+def is_cached(prefix: str, key_url: str, *, cache_dir: Path) -> bool:
+    """Check if content for the given prefix and URL is cached."""
+    key = (prefix, _key_for_url(key_url))
+    return _key_directory(cache_dir, key).is_dir()
+
+
 def _key_for_url(url: str | urllib.parse.ParseResult) -> str:
     """
     Return a cache key suitable to cache content retrieved from the given URL.

src/cjdk/_conf.py

@@ -1,6 +1,18 @@
 # This file is part of cjdk.
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
+
+"""
+Configuration management.
+
+Defines the Configuration dataclass, parses and validates parameters, detects
+platform (OS/architecture canonicalization), determines default cache
+directories (platform-specific), and resolves environment variable overrides
+(CJDK_*).
+
+No actual operations.
+"""
+
 from __future__ import annotations
 
 import os

src/cjdk/_download.py

@@ -1,6 +1,16 @@
 # This file is part of cjdk.
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
+
+"""
+HTTP downloads and archive extraction.
+
+Downloads files via HTTPS with progress tracking, extracts .zip and .tgz
+archives, and preserves executable bits from zip files.
+
+No JDK-specific or cache-related operations.
+"""
+
 from __future__ import annotations
 
 import sys

src/cjdk/_exceptions.py

@@ -2,6 +2,13 @@
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
 
+"""
+Exception hierarchy.
+
+Defines the base CjdkError and specific subclasses. These are exposed to the
+API by __init__.py.
+"""
+
 __all__ = [
     "CjdkError",
     "ConfigError",

src/cjdk/_index.py

@@ -1,6 +1,18 @@
 # This file is part of cjdk.
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
+
+"""
+JDK index handling.
+
+Fetches and caches the Coursier JDK index, parses JSON, normalizes vendor names
+(e.g., merges ibm-semeru-*-java## variants), and performs version
+matching/resolution with support for version expressions like "17+".
+
+No actual operations except for caching the index itself. _index should be
+considered an internal helper for _jdk and should not be used directly.
+"""
+
 from __future__ import annotations
 
 import copy
@@ -19,9 +31,9 @@
 
 __all__ = [
     "jdk_index",
-    "available_jdks",
-    "resolve_jdk_version",
     "jdk_url",
+    "matching_jdk_versions",
+    "resolve_jdk_version",
 ]
 
 
@@ -43,7 +55,9 @@ def jdk_index(conf: Configuration) -> Index:
     return _read_index(_cached_index_path(conf))
 
 
-def available_jdks(index: Index, conf: Configuration) -> list[tuple[str, str]]:
+def _available_jdks(
+    index: Index, conf: Configuration
+) -> list[tuple[str, str]]:
     """
     Find in index the available JDK vendor-version combinations.
 
@@ -71,7 +85,7 @@ def resolve_jdk_version(index: Index, conf: Configuration) -> str:
     Arguments:
     index -- The JDK index (nested dict)
     """
-    jdks = available_jdks(index, conf)
+    jdks = _available_jdks(index, conf)
     versions = _get_versions(jdks, conf)
     if not versions:
         raise JdkNotFoundError(
@@ -266,3 +280,43 @@ def _is_version_compatible_with_spec(
             and version[len(spec) - 1] >= spec[-1]
         )
     return len(version) >= len(spec) and version[: len(spec)] == spec
+
+
+class _VersionElement:
+    """Wrapper for version tuple elements enabling mixed int/str comparison."""
+
+    def __init__(self, value: int | str) -> None:
+        self.value = value
+
+    def __eq__(self, other: object) -> bool:
+        if not isinstance(other, _VersionElement):
+            return NotImplemented
+        if isinstance(self.value, int) and isinstance(other.value, int):
+            return self.value == other.value
+        return str(self.value) == str(other.value)
+
+    def __lt__(self, other: _VersionElement) -> bool:
+        if isinstance(self.value, int) and isinstance(other.value, int):
+            return self.value < other.value
+        return str(self.value) < str(other.value)
+
+
+def matching_jdk_versions(index: Index, conf: Configuration) -> list[str]:
+    """
+    Return all version strings matching the configuration, sorted by version.
+
+    Unlike resolve_jdk_version() which returns only the best match, this
+    returns all compatible versions.
+    """
+    jdks = _available_jdks(index, conf)
+    versions = _get_versions(jdks, conf)
+    if not versions:
+        return []
+    matched = _match_versions(conf.vendor, versions, conf.version)
+
+    def version_sort_key(
+        item: tuple[tuple[int | str, ...], str],
+    ) -> tuple[_VersionElement, ...]:
+        return tuple(_VersionElement(e) for e in item[0])
+
+    return [v for _, v in sorted(matched.items(), key=version_sort_key)]

src/cjdk/_install.py

@@ -1,6 +1,16 @@
 # This file is part of cjdk.
 # Copyright 2022-25 Board of Regents of the University of Wisconsin System
 # SPDX-License-Identifier: MIT
+
+"""
+Installation orchestration.
+
+Bridges _cache and _download: provides install_file and install_dir functions
+that coordinate downloading with caching, and prints progress headers.
+
+No JDK-specific operations.
+"""
+
 from __future__ import annotations
 
 import sys