From 24e9787802e0f29b077530851a7339d6320bf04c Mon Sep 17 00:00:00 2001 From: antazoey Date: Wed, 12 Jun 2024 18:27:32 -0500 Subject: [PATCH] fix: avoid duplicate caching with v versus non-v prefix in dependency versions (#2131) --- src/ape/managers/project.py | 26 ++++++++-- tests/functional/test_dependencies.py | 68 +++++++++++++++++++++++++++ 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/ape/managers/project.py b/src/ape/managers/project.py index e6513749b9..fd4fda0115 100644 --- a/src/ape/managers/project.py +++ b/src/ape/managers/project.py @@ -837,6 +837,26 @@ def _get_cache_suffix(package_id: str, version: str, suffix: str = "") -> Path: return package_id_path / version_name +def _get_cache_path( + base_path: Path, package_id: str, version: str, is_dir: bool = False, suffix: str = "" +) -> Path: + options = _version_to_options(version) + original = None + for option in options: + path = base_path / _get_cache_suffix(package_id, option, suffix=suffix) + + if original is None: + # The 'original' is the first option. + original = path + + if (is_dir and path.is_dir()) or (not is_dir and path.is_file()): + return path + + # Return original - may no be created yet! + assert original is not None # For mypy. + return original + + class PackagesCache(ManagerAccessMixin): def __init__(self): self._api_cache: dict[str, DependencyAPI] = {} @@ -875,21 +895,21 @@ def get_project_path(self, package_id: str, version: str) -> Path: """ Path to the dir of the cached project. """ - return self.projects_folder / _get_cache_suffix(package_id, version) + return _get_cache_path(self.projects_folder, package_id, version, is_dir=True) def get_manifest_path(self, package_id: str, version: str) -> Path: """ Path to the manifest filepath the dependency project uses as a base. """ - return self.manifests_folder / _get_cache_suffix(package_id, version, suffix=".json") + return _get_cache_path(self.manifests_folder, package_id, version, suffix=".json") def get_api_path(self, package_id: str, version: str) -> Path: """ Path to the manifest filepath the dependency project uses as a base. """ - return self.api_folder / _get_cache_suffix(package_id, version, suffix=".json") + return _get_cache_path(self.api_folder, package_id, version, suffix=".json") def cache_api(self, api: DependencyAPI) -> Path: """ diff --git a/tests/functional/test_dependencies.py b/tests/functional/test_dependencies.py index 398edccdac..3b89e9e063 100644 --- a/tests/functional/test_dependencies.py +++ b/tests/functional/test_dependencies.py @@ -323,6 +323,74 @@ def test_cache_api(self, cache): actual = json.loads(path.read_text()) assert actual == {"name": "depabc", "local": "depabc"} + def test_cache_api_when_v_prefix_exists(self, cache): + # First, cache the v-prefix + dep = LocalDependency(name="depabc", local=Path("depabc"), version="v1.0.0") + path0 = cache.cache_api(dep) + assert path0.is_file() + + # Now, try to cache again w/o v prefix + dep.version = "1.0.0" + path1 = cache.cache_api(dep) + assert path1 == path0 + + def test_cache_api_when_non_v_prefix_exists(self, cache): + # First, cache the non-v-prefix + dep = LocalDependency(name="depabc", local=Path("depabc"), version="1.0.0") + path0 = cache.cache_api(dep) + assert path0.is_file() + + # Now, try to cache again with the v prefix + dep.version = "v1.0.0" + path1 = cache.cache_api(dep) + assert path1 == path0 + + def test_get_manifest_path(self, cache, data_folder): + package_id = "this/is/my_package-ID" + version = "version12/5.54" + actual = cache.get_manifest_path(package_id, version) + expected = ( + data_folder / "packages" / "manifests" / "this_is_my_package-ID" / "version12_5_54.json" + ) + assert actual == expected + + def test_get_manifest_path_v_prefix_exists(self, cache, data_folder): + file = data_folder / "packages" / "manifests" / "manifest-pkg-test-1" / "v1_0_0.json" + file.parent.mkdir(parents=True, exist_ok=True) + file.touch() + + # Requesting w/o v-prefix. Should still work. + path = cache.get_manifest_path("manifest-pkg-test-1", "1.0.0") + assert path == file + + def test_get_manifest_path_non_v_prefix_exists(self, cache, data_folder): + file = data_folder / "packages" / "manifests" / "manifest-pkg-test-2" / "1_0_0.json" + file.parent.mkdir(parents=True, exist_ok=True) + file.touch() + + # Requesting w/o v-prefix. Should still work. + path = cache.get_manifest_path("manifest-pkg-test-2", "v1.0.0") + assert path == file + + def test_get_project_path(self, cache, data_folder): + path = data_folder / "packages" / "projects" / "project-test-1" / "local" + actual = cache.get_project_path("project-test-1", "local") + assert actual == path + + def test_get_project_path_missing_v_prefix(self, cache, data_folder): + path = data_folder / "packages" / "projects" / "project-test-1" / "v1_0_0" + path.mkdir(parents=True, exist_ok=True) + # Missing v-prefix on request. + actual = cache.get_project_path("project-test-1", "1.0.0") + assert actual == path + + def test_get_project_path_unneeded_v_prefix(self, cache, data_folder): + path = data_folder / "packages" / "projects" / "project-test-2" / "1_0_0" + path.mkdir(parents=True, exist_ok=True) + # Unneeded v-prefix on request. + actual = cache.get_project_path("project-test-2", "v1.0.0") + assert actual == path + class TestLocalDependency: NAME = "testlocaldep"