diff --git a/src/poetry/puzzle/provider.py b/src/poetry/puzzle/provider.py index baf04dc60ef..1e634995dd7 100644 --- a/src/poetry/puzzle/provider.py +++ b/src/poetry/puzzle/provider.py @@ -555,7 +555,7 @@ def complete_package( package.pretty_name, package.version, extras=list(dependency.extras), - repository=dependency.source_name, + repository_name=dependency.source_name, ), ) except PackageNotFound as e: diff --git a/src/poetry/repositories/abstract_repository.py b/src/poetry/repositories/abstract_repository.py new file mode 100644 index 00000000000..dbbc6a95026 --- /dev/null +++ b/src/poetry/repositories/abstract_repository.py @@ -0,0 +1,37 @@ +from __future__ import annotations + +from abc import ABC +from abc import abstractmethod +from typing import TYPE_CHECKING + + +if TYPE_CHECKING: + from poetry.core.constraints.version import Version + from poetry.core.packages.dependency import Dependency + from poetry.core.packages.package import Package + + +class AbstractRepository(ABC): + def __init__(self, name: str) -> None: + self._name = name + + @property + def name(self) -> str: + return self._name + + @abstractmethod + def find_packages(self, dependency: Dependency) -> list[Package]: + ... + + @abstractmethod + def search(self, query: str) -> list[Package]: + ... + + @abstractmethod + def package( + self, + name: str, + version: Version, + extras: list[str] | None = None, + ) -> Package: + ... diff --git a/src/poetry/repositories/cached.py b/src/poetry/repositories/cached.py index d3c6064d00e..59d6ba5fb71 100644 --- a/src/poetry/repositories/cached.py +++ b/src/poetry/repositories/cached.py @@ -46,7 +46,7 @@ def __init__( def _get_release_info( self, name: NormalizedName, version: Version ) -> dict[str, Any]: - raise NotImplementedError() + ... def get_release_info(self, name: NormalizedName, version: Version) -> PackageInfo: """ diff --git a/src/poetry/repositories/http.py b/src/poetry/repositories/http.py index 16bac1e4c1b..07315736d5a 100644 --- a/src/poetry/repositories/http.py +++ b/src/poetry/repositories/http.py @@ -5,7 +5,6 @@ import urllib import urllib.parse -from abc import ABC from collections import defaultdict from pathlib import Path from typing import TYPE_CHECKING @@ -35,7 +34,7 @@ from poetry.utils.authenticator import RepositoryCertificateConfig -class HTTPRepository(CachedRepository, ABC): +class HTTPRepository(CachedRepository): def __init__( self, name: str, diff --git a/src/poetry/repositories/legacy_repository.py b/src/poetry/repositories/legacy_repository.py index bc20b98d9a6..4823a0b71fd 100644 --- a/src/poetry/repositories/legacy_repository.py +++ b/src/poetry/repositories/legacy_repository.py @@ -33,6 +33,18 @@ def __init__( super().__init__(name, url.rstrip("/"), config, disable_cache) + @property + def packages(self) -> list[Package]: + # LegacyRepository._packages is not populated and other implementations + # implicitly rely on this (e.g. Pool.search via + # LegacyRepository.search). To avoid special-casing Pool or changing + # behavior, we stub and return an empty list. + # + # TODO: Rethinking search behaviour and design. + # Ref: https://github.com/python-poetry/poetry/issues/2446 and + # https://github.com/python-poetry/poetry/pull/6669#discussion_r990874908. + return [] + def package( self, name: str, version: Version, extras: list[str] | None = None ) -> Package: diff --git a/src/poetry/repositories/pool.py b/src/poetry/repositories/pool.py index 43f1e9d2a06..8a345b16cfc 100644 --- a/src/poetry/repositories/pool.py +++ b/src/poetry/repositories/pool.py @@ -1,9 +1,14 @@ from __future__ import annotations +import enum + +from collections import OrderedDict +from dataclasses import dataclass +from enum import IntEnum from typing import TYPE_CHECKING +from poetry.repositories.abstract_repository import AbstractRepository from poetry.repositories.exceptions import PackageNotFound -from poetry.repositories.repository import Repository if TYPE_CHECKING: @@ -11,50 +16,65 @@ from poetry.core.packages.dependency import Dependency from poetry.core.packages.package import Package + from poetry.repositories.repository import Repository + + +class Priority(IntEnum): + # The order of the members below dictates the actual priority. The first member has + # top priority. + DEFAULT = enum.auto() + PRIMARY = enum.auto() + SECONDARY = enum.auto() + + +@dataclass(frozen=True) +class PrioritizedRepository: + repository: Repository + priority: Priority -class Pool(Repository): + +class Pool(AbstractRepository): def __init__( self, repositories: list[Repository] | None = None, ignore_repository_names: bool = False, ) -> None: super().__init__("poetry-pool") + self._repositories: OrderedDict[str, PrioritizedRepository] = OrderedDict() + self._ignore_repository_names = ignore_repository_names if repositories is None: repositories = [] - - self._lookup: dict[str, int] = {} - self._repositories: list[Repository] = [] - self._default = False - self._has_primary_repositories = False - self._secondary_start_idx: int | None = None - for repository in repositories: self.add_repository(repository) - self._ignore_repository_names = ignore_repository_names - @property def repositories(self) -> list[Repository]: - return self._repositories + unsorted_repositories = self._repositories.values() + sorted_repositories = sorted( + unsorted_repositories, key=lambda prio_repo: prio_repo.priority + ) + return [prio_repo.repository for prio_repo in sorted_repositories] def has_default(self) -> bool: - return self._default + return self._contains_priority(Priority.DEFAULT) def has_primary_repositories(self) -> bool: - return self._has_primary_repositories + return self._contains_priority(Priority.PRIMARY) + + def _contains_priority(self, priority: Priority) -> bool: + return any( + prio_repo.priority is priority for prio_repo in self._repositories.values() + ) def has_repository(self, name: str) -> bool: - return name.lower() in self._lookup + return name.lower() in self._repositories def repository(self, name: str) -> Repository: name = name.lower() - - lookup = self._lookup.get(name) - if lookup is not None: - return self._repositories[lookup] - - raise ValueError(f'Repository "{name}" does not exist.') + if self.has_repository(name): + return self._repositories[name].repository + raise IndexError(f'Repository "{name}" does not exist.') def add_repository( self, repository: Repository, default: bool = False, secondary: bool = False @@ -63,133 +83,61 @@ def add_repository( Adds a repository to the pool. """ repository_name = repository.name.lower() - if repository_name in self._lookup: - raise ValueError(f"{repository_name} already added") - - if default: - if self.has_default(): - raise ValueError("Only one repository can be the default") - - self._default = True - self._repositories.insert(0, repository) - for name in self._lookup: - self._lookup[name] += 1 + if self.has_repository(repository_name): + raise ValueError( + f"A repository with name {repository_name} was already added." + ) - if self._secondary_start_idx is not None: - self._secondary_start_idx += 1 + if default and self.has_default(): + raise ValueError("Only one repository can be the default.") - self._lookup[repository_name] = 0 + priority = Priority.PRIMARY + if default: + priority = Priority.DEFAULT elif secondary: - if self._secondary_start_idx is None: - self._secondary_start_idx = len(self._repositories) - - self._repositories.append(repository) - self._lookup[repository_name] = len(self._repositories) - 1 - else: - self._has_primary_repositories = True - if self._secondary_start_idx is None: - self._repositories.append(repository) - self._lookup[repository_name] = len(self._repositories) - 1 - else: - self._repositories.insert(self._secondary_start_idx, repository) - - for name, idx in self._lookup.items(): - if idx < self._secondary_start_idx: - continue - - self._lookup[name] += 1 - - self._lookup[repository_name] = self._secondary_start_idx - self._secondary_start_idx += 1 - + priority = Priority.SECONDARY + self._repositories[repository_name] = PrioritizedRepository( + repository, priority + ) return self - def remove_repository(self, repository_name: str) -> Pool: - if repository_name is not None: - repository_name = repository_name.lower() - - idx = self._lookup.get(repository_name) - if idx is not None: - del self._repositories[idx] - del self._lookup[repository_name] - - if idx == 0: - self._default = False - - for name in self._lookup: - if self._lookup[name] > idx: - self._lookup[name] -= 1 - - if ( - self._secondary_start_idx is not None - and self._secondary_start_idx > idx - ): - self._secondary_start_idx -= 1 - + def remove_repository(self, name: str) -> Pool: + if not self.has_repository(name): + raise IndexError(f"Pool can not remove unknown repository '{name}'.") + del self._repositories[name.lower()] return self - def has_package(self, package: Package) -> bool: - raise NotImplementedError() - def package( self, name: str, version: Version, extras: list[str] | None = None, - repository: str | None = None, + repository_name: str | None = None, ) -> Package: - if repository is not None: - repository = repository.lower() - - if ( - repository is not None - and repository not in self._lookup - and not self._ignore_repository_names - ): - raise ValueError(f'Repository "{repository}" does not exist.') + if repository_name and not self._ignore_repository_names: + return self.repository(repository_name).package( + name, version, extras=extras + ) - if repository is not None and not self._ignore_repository_names: - return self.repository(repository).package(name, version, extras=extras) - - for repo in self._repositories: + for repo in self.repositories: try: - package = repo.package(name, version, extras=extras) + return repo.package(name, version, extras=extras) except PackageNotFound: continue - - return package - raise PackageNotFound(f"Package {name} ({version}) not found.") def find_packages(self, dependency: Dependency) -> list[Package]: - repository = dependency.source_name - if repository is not None: - repository = repository.lower() - - if ( - repository is not None - and repository not in self._lookup - and not self._ignore_repository_names - ): - raise ValueError(f'Repository "{repository}" does not exist.') - - if repository is not None and not self._ignore_repository_names: - return self.repository(repository).find_packages(dependency) - - packages = [] - for repo in self._repositories: - packages += repo.find_packages(dependency) + repository_name = dependency.source_name + if repository_name and not self._ignore_repository_names: + return self.repository(repository_name).find_packages(dependency) + packages: list[Package] = [] + for repo in self.repositories: + packages += repo.find_packages(dependency) return packages def search(self, query: str) -> list[Package]: - from poetry.repositories.legacy_repository import LegacyRepository - - results = [] - for repository in self._repositories: - if isinstance(repository, LegacyRepository): - continue - + results: list[Package] = [] + for repository in self.repositories: results += repository.search(query) - return results diff --git a/src/poetry/repositories/repository.py b/src/poetry/repositories/repository.py index ee54362af55..46ecaac7475 100644 --- a/src/poetry/repositories/repository.py +++ b/src/poetry/repositories/repository.py @@ -8,6 +8,7 @@ from poetry.core.constraints.version import Version from poetry.core.constraints.version import VersionRange +from poetry.repositories.abstract_repository import AbstractRepository from poetry.repositories.exceptions import PackageNotFound @@ -19,18 +20,14 @@ from poetry.core.packages.utils.link import Link -class Repository: +class Repository(AbstractRepository): def __init__(self, name: str, packages: list[Package] | None = None) -> None: - self._name = name + super().__init__(name) self._packages: list[Package] = [] for package in packages or []: self.add_package(package) - @property - def name(self) -> str: - return self._name - @property def packages(self) -> list[Package]: return self._packages diff --git a/tests/console/commands/test_add.py b/tests/console/commands/test_add.py index fedaacc020e..7947d209d8c 100644 --- a/tests/console/commands/test_add.py +++ b/tests/console/commands/test_add.py @@ -858,7 +858,7 @@ def test_add_constraint_with_source( def test_add_constraint_with_source_that_does_not_exist( app: PoetryTestApplication, tester: CommandTester ): - with pytest.raises(ValueError) as e: + with pytest.raises(IndexError) as e: tester.execute("foo --source i-dont-exist") assert str(e.value) == 'Repository "i-dont-exist" does not exist.' @@ -1848,7 +1848,7 @@ def test_add_constraint_with_source_old_installer( def test_add_constraint_with_source_that_does_not_exist_old_installer( app: PoetryTestApplication, old_tester: CommandTester ): - with pytest.raises(ValueError) as e: + with pytest.raises(IndexError) as e: old_tester.execute("foo --source i-dont-exist") assert str(e.value) == 'Repository "i-dont-exist" does not exist.' diff --git a/tests/repositories/test_legacy_repository.py b/tests/repositories/test_legacy_repository.py index fb96c65b1cd..ab8f7022654 100644 --- a/tests/repositories/test_legacy_repository.py +++ b/tests/repositories/test_legacy_repository.py @@ -63,6 +63,13 @@ def _download(self, url: str, dest: Path) -> None: shutil.copyfile(str(filepath), dest) +def test_packages_property_returns_empty_list() -> None: + repo = MockRepository() + repo._packages = [repo.package("jupyter", Version.parse("1.0.0"))] + + assert repo.packages == [] + + def test_page_relative_links_path_are_correct() -> None: repo = MockRepository() diff --git a/tests/repositories/test_pool.py b/tests/repositories/test_pool.py index 84f18c4ea56..dc68bf0015b 100644 --- a/tests/repositories/test_pool.py +++ b/tests/repositories/test_pool.py @@ -8,39 +8,46 @@ from poetry.repositories import Repository from poetry.repositories.exceptions import PackageNotFound from poetry.repositories.legacy_repository import LegacyRepository +from tests.helpers import get_dependency +from tests.helpers import get_package -def test_pool_raises_package_not_found_when_no_package_is_found() -> None: - pool = Pool() - pool.add_repository(Repository("repo")) - - with pytest.raises(PackageNotFound): - pool.package("foo", Version.parse("1.0.0")) - - -def test_pool(): +def test_pool() -> None: pool = Pool() assert len(pool.repositories) == 0 assert not pool.has_default() + assert not pool.has_primary_repositories() -def test_pool_with_initial_repositories(): +def test_pool_with_initial_repositories() -> None: repo = Repository("repo") pool = Pool([repo]) assert len(pool.repositories) == 1 assert not pool.has_default() + assert pool.has_primary_repositories() -def test_repository_no_repository(): +def test_repository_no_repository() -> None: pool = Pool() - with pytest.raises(ValueError): + with pytest.raises(IndexError): pool.repository("foo") -def test_repository_from_normal_pool(): +def test_adding_repositories_with_same_name_twice_raises_value_error() -> None: + repo1 = Repository("repo") + repo2 = Repository("repo") + + with pytest.raises(ValueError): + Pool([repo1, repo2]) + + with pytest.raises(ValueError): + Pool([repo1]).add_repository(repo2) + + +def test_repository_from_normal_pool() -> None: repo = LegacyRepository("foo", "https://foo.bar") pool = Pool() pool.add_repository(repo) @@ -48,7 +55,7 @@ def test_repository_from_normal_pool(): assert pool.repository("foo") is repo -def test_repository_from_secondary_pool(): +def test_repository_from_secondary_pool() -> None: repo = LegacyRepository("foo", "https://foo.bar") pool = Pool() pool.add_repository(repo, secondary=True) @@ -56,7 +63,7 @@ def test_repository_from_secondary_pool(): assert pool.repository("foo") is repo -def test_repository_with_normal_default_and_secondary_repositories(): +def test_repository_with_normal_default_and_secondary_repositories() -> None: secondary = LegacyRepository("secondary", "https://secondary.com") default = LegacyRepository("default", "https://default.com") repo1 = LegacyRepository("foo", "https://foo.bar") @@ -73,9 +80,17 @@ def test_repository_with_normal_default_and_secondary_repositories(): assert pool.repository("foo") is repo1 assert pool.repository("bar") is repo2 assert pool.has_default() + assert pool.has_primary_repositories() -def test_remove_repository(): +def test_remove_non_existing_repository_raises_indexerror() -> None: + pool = Pool() + + with pytest.raises(IndexError): + pool.remove_repository("foo") + + +def test_remove_existing_repository_successful() -> None: repo1 = LegacyRepository("foo", "https://foo.bar") repo2 = LegacyRepository("bar", "https://bar.baz") repo3 = LegacyRepository("baz", "https://baz.quux") @@ -91,7 +106,7 @@ def test_remove_repository(): assert pool.repository("baz") is repo3 -def test_remove_default_repository(): +def test_remove_default_repository() -> None: default = LegacyRepository("default", "https://default.com") repo1 = LegacyRepository("foo", "https://foo.bar") repo2 = LegacyRepository("bar", "https://bar.baz") @@ -115,7 +130,7 @@ def test_remove_default_repository(): assert not pool.has_repository("default") -def test_repository_ordering(): +def test_repository_ordering() -> None: default1 = LegacyRepository("default1", "https://default1.com") default2 = LegacyRepository("default2", "https://default2.com") primary1 = LegacyRepository("primary1", "https://primary1.com") @@ -141,3 +156,113 @@ def test_repository_ordering(): assert pool.repositories == [default1, primary1, primary3, secondary1, secondary3] with pytest.raises(ValueError): pool.add_repository(default2, default=True) + + +def test_pool_get_package_in_any_repository() -> None: + package1 = get_package("foo", "1.0.0") + repo1 = Repository("repo1", [package1]) + package2 = get_package("bar", "1.0.0") + repo2 = Repository("repo2", [package1, package2]) + pool = Pool([repo1, repo2]) + + returned_package1 = pool.package("foo", Version.parse("1.0.0")) + returned_package2 = pool.package("bar", Version.parse("1.0.0")) + + assert returned_package1 == package1 + assert returned_package2 == package2 + + +def test_pool_get_package_in_specified_repository() -> None: + package = get_package("foo", "1.0.0") + repo1 = Repository("repo1") + repo2 = Repository("repo2", [package]) + pool = Pool([repo1, repo2]) + + returned_package = pool.package( + "foo", Version.parse("1.0.0"), repository_name="repo2" + ) + + assert returned_package == package + + +def test_pool_no_package_from_any_repository_raises_package_not_found() -> None: + pool = Pool() + pool.add_repository(Repository("repo")) + + with pytest.raises(PackageNotFound): + pool.package("foo", Version.parse("1.0.0")) + + +def test_pool_no_package_from_specified_repository_raises_package_not_found() -> None: + package = get_package("foo", "1.0.0") + repo1 = Repository("repo1") + repo2 = Repository("repo2", [package]) + pool = Pool([repo1, repo2]) + + with pytest.raises(PackageNotFound): + pool.package("foo", Version.parse("1.0.0"), repository_name="repo1") + + +def test_pool_find_packages_in_any_repository() -> None: + package1 = get_package("foo", "1.1.1") + package2 = get_package("foo", "1.2.3") + package3 = get_package("foo", "2.0.0") + package4 = get_package("bar", "1.2.3") + repo1 = Repository("repo1", [package1, package3]) + repo2 = Repository("repo2", [package1, package2, package4]) + pool = Pool([repo1, repo2]) + + available_dependency = get_dependency("foo", "^1.0.0") + returned_packages_available = pool.find_packages(available_dependency) + unavailable_dependency = get_dependency("foo", "999.9.9") + returned_packages_unavailable = pool.find_packages(unavailable_dependency) + + assert returned_packages_available == [package1, package1, package2] + assert returned_packages_unavailable == [] + + +def test_pool_find_packages_in_specified_repository() -> None: + package_foo1 = get_package("foo", "1.1.1") + package_foo2 = get_package("foo", "1.2.3") + package_foo3 = get_package("foo", "2.0.0") + package_bar = get_package("bar", "1.2.3") + repo1 = Repository("repo1", [package_foo1, package_foo3]) + repo2 = Repository("repo2", [package_foo1, package_foo2, package_bar]) + pool = Pool([repo1, repo2]) + + available_dependency = get_dependency("foo", "^1.0.0") + available_dependency.source_name = "repo2" + returned_packages_available = pool.find_packages(available_dependency) + unavailable_dependency = get_dependency("foo", "999.9.9") + unavailable_dependency.source_name = "repo2" + returned_packages_unavailable = pool.find_packages(unavailable_dependency) + + assert returned_packages_available == [package_foo1, package_foo2] + assert returned_packages_unavailable == [] + + +def test_search_no_legacy_repositories() -> None: + package_foo1 = get_package("foo", "1.0.0") + package_foo2 = get_package("foo", "2.0.0") + package_foobar = get_package("foobar", "1.0.0") + repo1 = Repository("repo1", [package_foo1, package_foo2]) + repo2 = Repository("repo2", [package_foo1, package_foobar]) + pool = Pool([repo1, repo2]) + + assert pool.search("foo") == [ + package_foo1, + package_foo2, + package_foo1, + package_foobar, + ] + assert pool.search("bar") == [package_foobar] + assert pool.search("nothing") == [] + + +def test_search_legacy_repositories_are_skipped() -> None: + package = get_package("foo", "1.0.0") + repo1 = Repository("repo1", [package]) + repo2 = LegacyRepository("repo2", "https://fake.repo/") + pool = Pool([repo1, repo2]) + + assert pool.search("foo") == [package] diff --git a/tests/repositories/test_repository.py b/tests/repositories/test_repository.py index d2ff270b10a..551691d9ff7 100644 --- a/tests/repositories/test_repository.py +++ b/tests/repositories/test_repository.py @@ -3,17 +3,17 @@ import pytest from poetry.core.constraints.version import Version -from poetry.core.packages.package import Package from poetry.factory import Factory from poetry.repositories import Repository +from tests.helpers import get_package @pytest.fixture(scope="module") def black_repository() -> Repository: repo = Repository("repo") - repo.add_package(Package("black", "19.10b0")) - repo.add_package(Package("black", "21.11b0", yanked="reason")) + repo.add_package(get_package("black", "19.10b0")) + repo.add_package(get_package("black", "21.11b0", yanked="reason")) return repo @@ -63,7 +63,18 @@ def test_package_yanked( def test_package_pretty_name_is_kept() -> None: pretty_name = "Not_canoni-calized.name" repo = Repository("repo") - repo.add_package(Package(pretty_name, "1.0")) + repo.add_package(get_package(pretty_name, "1.0")) package = repo.package(pretty_name, Version.parse("1.0")) assert package.pretty_name == pretty_name + + +def test_search() -> None: + package_foo1 = get_package("foo", "1.0.0") + package_foo2 = get_package("foo", "2.0.0") + package_foobar = get_package("foobar", "1.0.0") + repo = Repository("repo", [package_foo1, package_foo2, package_foobar]) + + assert repo.search("foo") == [package_foo1, package_foo2, package_foobar] + assert repo.search("bar") == [package_foobar] + assert repo.search("nothing") == [] diff --git a/tests/test_factory.py b/tests/test_factory.py index 5671fdc5edf..7eafc85d210 100644 --- a/tests/test_factory.py +++ b/tests/test_factory.py @@ -299,7 +299,7 @@ def test_poetry_with_two_default_sources(with_simple_keyring: None): with pytest.raises(ValueError) as e: Factory().create_poetry(fixtures_dir / "with_two_default_sources") - assert str(e.value) == "Only one repository can be the default" + assert str(e.value) == "Only one repository can be the default." def test_validate():