cache: automatic refresh if files of locked package differ from files in cache (#10657)

When working on a project with many developers, unnecessary diffs in the lockfile due to outdated caches of some developers are an annoying issue. Outdated caches are normally the result of postponed artifact uploads. This change detects such diffs and refreshes the cache automatically.

Author: radoering

src/poetry/puzzle/provider.py

@@ -151,6 +151,7 @@ def __init__(
             )
 
         self.get_package_from_pool = functools.cache(self._pool.package)
+        self._refreshed: set[tuple[str, Version, str | None]] = set()
 
     @property
     def pool(self) -> RepositoryPool:
@@ -471,14 +472,34 @@ def complete_package(
         elif package.is_direct_origin():
             requires = package.requires
         else:
-            dependency_package = DependencyPackage(
-                dependency,
-                self.get_package_from_pool(
+            if (
+                package.pretty_name,
+                package.version,
+                dependency.source_name,
+            ) in self._refreshed:
+                # circumvent lru_cache to avoid unnecessary refresh
+                pool_package = self.pool.package(
                     package.pretty_name,
                     package.version,
                     repository_name=dependency.source_name,
-                ),
-            )
+                )
+            else:
+                pool_package = self.get_package_from_pool(
+                    package.pretty_name,
+                    package.version,
+                    repository_name=dependency.source_name,
+                )
+            if package.files and sorted(
+                package.files, key=lambda f: f["file"]
+            ) != sorted(pool_package.files, key=lambda f: f["file"]):
+                # This happens if additional artifacts are uploaded later. Either our own cache
+                # is outdated or the lockfile has been created with an outdated cache.
+                # Refresh to cover the first case. (It does not hurt much in the second case.)
+                pool_package = self.pool.refresh(pool_package)
+                self._refreshed.add(
+                    (package.pretty_name, package.version, dependency.source_name)
+                )
+            dependency_package = DependencyPackage(dependency, pool_package)
 
             package = dependency_package.package
             dependency = dependency_package.dependency

src/poetry/repositories/cached_repository.py

@@ -70,3 +70,6 @@ def package(self, name: str, version: Version) -> Package:
         return self.get_release_info(canonicalize_name(name), version).to_package(
             name=name
         )
+
+    def forget(self, name: str, version: Version) -> None:
+        self._release_cache.forget(f"{canonicalize_name(name)}:{version}")

src/poetry/repositories/repository_pool.py

@@ -9,6 +9,7 @@
 
 from poetry.config.config import Config
 from poetry.repositories.abstract_repository import AbstractRepository
+from poetry.repositories.cached_repository import CachedRepository
 from poetry.repositories.exceptions import PackageNotFoundError
 from poetry.repositories.repository import Repository
 from poetry.utils.cache import ArtifactCache
@@ -181,3 +182,10 @@ def search(self, query: str | list[str]) -> list[Package]:
         for repo in self.repositories:
             results += repo.search(query)
         return results
+
+    def refresh(self, package: Package) -> Package:
+        repository_name = package.source_reference or "PyPI"
+        repo = self.repository(repository_name)
+        if isinstance(repo, CachedRepository):
+            repo.forget(package.name, package.version)
+        return repo.package(package.name, package.version)

tests/console/commands/test_add.py

@@ -1854,6 +1854,14 @@ def test_add_does_not_update_locked_dependencies(
     for package in docker_locked, docker_new, foo:
         repo.add_package(package)
 
+    # set correct files to avoid cache refresh
+    if locked:
+        docker_locked.files = (
+            poetry_with_up_to_date_lockfile.locker.locked_repository()
+            .package("docker", Version.parse("4.3.1"))
+            .files
+        )
+
     tester.execute(command)
 
     lock_data = poetry_with_up_to_date_lockfile.locker.lock_data

tests/console/commands/test_lock.py

@@ -5,6 +5,8 @@
 
 import pytest
 
+from poetry.core.constraints.version import Version
+
 from poetry.packages import Locker
 from tests.helpers import get_package
 
@@ -92,7 +94,8 @@ def test_lock_does_not_update_if_not_necessary(
     poetry_with_old_lockfile: Poetry,
     repo: TestRepository,
 ) -> None:
-    repo.add_package(get_package("sampleproject", "1.3.1"))
+    package = get_package("sampleproject", "1.3.1")
+    repo.add_package(package)
     repo.add_package(get_package("sampleproject", "2.0.0"))
 
     locker = Locker(
@@ -107,6 +110,13 @@ def test_lock_does_not_update_if_not_necessary(
         == "1.0"
     )
 
+    # set correct files to avoid cache refresh
+    package.files = (
+        locker.locked_repository()
+        .package("sampleproject", Version.parse("1.3.1"))
+        .files
+    )
+
     tester = command_tester_factory("lock", poetry=poetry_with_old_lockfile)
     tester.execute()
 

tests/puzzle/test_provider.py

@@ -10,6 +10,7 @@
 import pytest
 
 from cleo.io.null_io import NullIO
+from packaging.utils import NormalizedName
 from packaging.utils import canonicalize_name
 from poetry.core.packages.dependency import Dependency
 from poetry.core.packages.directory_dependency import DirectoryDependency
@@ -24,6 +25,7 @@
 from poetry.packages import DependencyPackage
 from poetry.puzzle.provider import IncompatibleConstraintsError
 from poetry.puzzle.provider import Provider
+from poetry.repositories.cached_repository import CachedRepository
 from poetry.repositories.exceptions import PackageNotFoundError
 from poetry.repositories.repository import Repository
 from poetry.repositories.repository_pool import Priority
@@ -36,6 +38,7 @@
 if TYPE_CHECKING:
     from pathlib import Path
 
+    from poetry.core.constraints.version import Version
     from pytest_mock import MockerFixture
 
     from tests.types import FixtureDirGetter
@@ -49,6 +52,18 @@ def run(self, bin: str, *args: str, **kwargs: Any) -> str:
         raise EnvCommandError(CalledProcessError(1, "python", output=""))
 
 
+class MockCachedRepository(CachedRepository):
+    def _get_release_info(
+        self, name: NormalizedName, version: Version
+    ) -> dict[str, Any]:
+        raise NotImplementedError
+
+    def package(self, name: str, version: Version) -> Package:
+        package = super().package(name, version)
+        package._source_reference = self.name
+        return package
+
+
 @pytest.fixture
 def root() -> ProjectPackage:
     return ProjectPackage("root", "1.2.3")
@@ -72,6 +87,46 @@ def provider(root: ProjectPackage, pool: RepositoryPool) -> Provider:
     return Provider(root, pool, NullIO())
 
 
+@pytest.fixture
+def release_info() -> PackageInfo:
+    return PackageInfo(
+        name="mylib",
+        version="1.0",
+        summary="",
+        requires_dist=[],
+        requires_python=">=3.9",
+        files=[
+            {
+                "file": "mylib-1.0-py3-none-any.whl",
+                "hash": "sha256:dummyhashvalue1234567890abcdef",
+            },
+            {
+                "file": "mylib-1.0.tar.gz",
+                "hash": "sha256:anotherdummyhashvalueabcdef1234567890",
+            },
+        ],
+        cache_version=str(CachedRepository.CACHE_VERSION),
+    )
+
+
+@pytest.fixture
+def outdated_release_info() -> PackageInfo:
+    return PackageInfo(
+        name="mylib",
+        version="1.0",
+        summary="",
+        requires_dist=[],
+        requires_python=">=3.9",
+        files=[
+            {
+                "file": "mylib-1.0-py3-none-any.whl",
+                "hash": "sha256:dummyhashvalue1234567890abcdef",
+            }
+        ],
+        cache_version=str(CachedRepository.CACHE_VERSION),
+    )
+
+
 @pytest.mark.parametrize(
     "dependency, expected",
     [
@@ -856,6 +911,43 @@ def test_complete_package_raises_packagenotfound_if_locked_source_not_available(
         provider.complete_package(locked)
 
 
+def test_complete_package_outdated_cache(
+    root: ProjectPackage,
+    release_info: PackageInfo,
+    outdated_release_info: PackageInfo,
+    mocker: MockerFixture,
+) -> None:
+    repo = MockCachedRepository("repo")
+    pool = RepositoryPool()
+    pool.add_repository(repo)
+    provider = Provider(root, pool, NullIO())
+    pool_refresh_spy = mocker.spy(provider.pool, "refresh")
+
+    assert release_info.name is not None
+    assert release_info.version is not None
+    package = Package(release_info.name, release_info.version)
+    package.files = release_info.files  # up-to-date files from lock
+
+    # trigger caching of outdated info
+    repo._get_release_info = lambda name, version: outdated_release_info.asdict()  # type: ignore[method-assign]
+    assert len(repo.package(package.name, package.version).files) == 1
+
+    # additional files uploaded -> refresh needed
+    repo._get_release_info = lambda name, version: release_info.asdict()  # type: ignore[method-assign]
+    complete_package = provider.complete_package(
+        DependencyPackage(package.to_dependency(), package)
+    )
+    assert len(complete_package.package.files) == 2
+    assert pool_refresh_spy.call_count == 1
+
+    # cache should have been updated -> no additional refresh needed
+    complete_package = provider.complete_package(
+        DependencyPackage(package.to_dependency(), package)
+    )
+    assert len(complete_package.package.files) == 2
+    assert pool_refresh_spy.call_count == 1
+
+
 def test_source_dependency_is_satisfied_by_direct_origin(
     provider: Provider, repository: Repository
 ) -> None:

tests/repositories/test_cached_repository.py

@@ -0,0 +1,82 @@
+from __future__ import annotations
+
+from typing import Any
+
+import pytest
+
+from packaging.utils import NormalizedName
+from packaging.utils import canonicalize_name
+from poetry.core.constraints.version import Version
+
+from poetry.inspection.info import PackageInfo
+from poetry.repositories.cached_repository import CachedRepository
+
+
+class MockCachedRepository(CachedRepository):
+    def _get_release_info(
+        self, name: NormalizedName, version: Version
+    ) -> dict[str, Any]:
+        raise NotImplementedError
+
+
+@pytest.fixture
+def release_info() -> PackageInfo:
+    return PackageInfo(
+        name="mylib",
+        version="1.0",
+        summary="",
+        requires_dist=[],
+        requires_python=">=3.9",
+        files=[
+            {
+                "file": "mylib-1.0-py3-none-any.whl",
+                "hash": "sha256:dummyhashvalue1234567890abcdef",
+            },
+            {
+                "file": "mylib-1.0.tar.gz",
+                "hash": "sha256:anotherdummyhashvalueabcdef1234567890",
+            },
+        ],
+        cache_version=str(CachedRepository.CACHE_VERSION),
+    )
+
+
+@pytest.fixture
+def outdated_release_info() -> PackageInfo:
+    return PackageInfo(
+        name="mylib",
+        version="1.0",
+        summary="",
+        requires_dist=[],
+        requires_python=">=3.9",
+        files=[
+            {
+                "file": "mylib-1.0-py3-none-any.whl",
+                "hash": "sha256:dummyhashvalue1234567890abcdef",
+            }
+        ],
+        cache_version=str(CachedRepository.CACHE_VERSION),
+    )
+
+
+@pytest.mark.parametrize("disable_cache", [False, True])
+def test_get_release_info_cache(
+    release_info: PackageInfo, outdated_release_info: PackageInfo, disable_cache: bool
+) -> None:
+    repo = MockCachedRepository("mock", disable_cache=disable_cache)
+    repo._get_release_info = lambda name, version: outdated_release_info.asdict()  # type: ignore[method-assign]
+
+    name = canonicalize_name("mylib")
+    version = Version.parse("1.0")
+    assert len(repo.get_release_info(name, version).files) == 1
+
+    # without disable_cache: cached value is returned even if the underlying data has changed
+    # with disable_cache: cached value is ignored and updated data is returned
+    repo._get_release_info = lambda name, version: release_info.asdict()  # type: ignore[method-assign]
+    assert len(repo.get_release_info(name, version).files) == (
+        2 if disable_cache else 1
+    )
+
+    # after clearing the cache entry, updated data is returned
+    repo.forget(name, version)
+    assert len(repo.get_release_info(name, version).files) == 2