Skip to content

Commit

Permalink
fix: avoid duplicate caching with v versus non-v prefix in dependency…
Browse files Browse the repository at this point in the history
… versions (ApeWorX#2131)
  • Loading branch information
antazoey authored Jun 12, 2024
1 parent 8ad275d commit 24e9787
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 3 deletions.
26 changes: 23 additions & 3 deletions src/ape/managers/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -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] = {}
Expand Down Expand Up @@ -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:
"""
Expand Down
68 changes: 68 additions & 0 deletions tests/functional/test_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down

0 comments on commit 24e9787

Please sign in to comment.