From 18ebe9619b483b097a4a8712af1c4d222e92e8c8 Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Sun, 9 Oct 2022 20:37:56 -0600 Subject: [PATCH 1/6] tests: 100% coverage for test_repository --- tests/repositories/test_pool.py | 157 +++++++++++++++++++++++--- tests/repositories/test_repository.py | 21 +++- 2 files changed, 156 insertions(+), 22 deletions(-) diff --git a/tests/repositories/test_pool.py b/tests/repositories/test_pool.py index 84f18c4ea56..9d660ad7b46 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): 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,111 @@ 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="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="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..83a7287cce4 100644 --- a/tests/repositories/test_repository.py +++ b/tests/repositories/test_repository.py @@ -2,18 +2,18 @@ import pytest -from poetry.core.constraints.version import Version -from poetry.core.packages.package import Package +from poetry.core.semver.version import Version 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") == [] From de4fcdf3e5a953fcc843e897a7e0384fc4153fa6 Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Fri, 30 Sep 2022 23:26:39 +0200 Subject: [PATCH 2/6] refactor: Pool should not inherit from Repository --- src/poetry/repositories/pool.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/poetry/repositories/pool.py b/src/poetry/repositories/pool.py index 43f1e9d2a06..3dfacd66b9e 100644 --- a/src/poetry/repositories/pool.py +++ b/src/poetry/repositories/pool.py @@ -3,7 +3,6 @@ from typing import TYPE_CHECKING from poetry.repositories.exceptions import PackageNotFound -from poetry.repositories.repository import Repository if TYPE_CHECKING: @@ -11,14 +10,16 @@ from poetry.core.packages.dependency import Dependency from poetry.core.packages.package import Package + from poetry.repositories.repository import Repository -class Pool(Repository): + +class Pool: def __init__( self, repositories: list[Repository] | None = None, ignore_repository_names: bool = False, ) -> None: - super().__init__("poetry-pool") + self._name = "poetry-pool" if repositories is None: repositories = [] @@ -34,6 +35,10 @@ def __init__( self._ignore_repository_names = ignore_repository_names + @property + def name(self) -> str: + return self._name + @property def repositories(self) -> list[Repository]: return self._repositories @@ -128,9 +133,6 @@ def remove_repository(self, repository_name: str) -> Pool: return self - def has_package(self, package: Package) -> bool: - raise NotImplementedError() - def package( self, name: str, From 1f94c4b7d551882fe7ea2a3eb0a89054f3b8cbc1 Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Fri, 30 Sep 2022 23:39:20 +0200 Subject: [PATCH 3/6] refactor: simplify Pool logic Introduces Priority enum to help in bookkeeping of repository types. Additionally specifies parameter 'repository' to 'repository_name' where relevant. --- src/poetry/puzzle/provider.py | 2 +- src/poetry/repositories/legacy_repository.py | 12 ++ src/poetry/repositories/pool.py | 180 +++++++------------ tests/console/commands/test_add.py | 4 +- tests/repositories/test_legacy_repository.py | 7 + tests/repositories/test_pool.py | 8 +- tests/test_factory.py | 2 +- 7 files changed, 88 insertions(+), 127 deletions(-) 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/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 3dfacd66b9e..419d8990c2d 100644 --- a/src/poetry/repositories/pool.py +++ b/src/poetry/repositories/pool.py @@ -1,5 +1,9 @@ from __future__ import annotations +import enum + +from collections import OrderedDict +from enum import IntEnum from typing import TYPE_CHECKING from poetry.repositories.exceptions import PackageNotFound @@ -13,6 +17,14 @@ 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() + + class Pool: def __init__( self, @@ -20,46 +32,45 @@ def __init__( ignore_repository_names: bool = False, ) -> None: self._name = "poetry-pool" + self._repositories: OrderedDict[ + str, tuple[Repository, RepositoryPriority] + ] = 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 name(self) -> str: return self._name @property def repositories(self) -> list[Repository]: - return self._repositories + unsorted_repositories = self._repositories.values() + sorted_repositories = sorted(unsorted_repositories, key=lambda p: p[1].value) + return [repo for (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( + repo_prio is priority for _, repo_prio 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][0] + raise IndexError(f'Repository "{name}" does not exist.') def add_repository( self, repository: Repository, default: bool = False, secondary: bool = False @@ -68,69 +79,26 @@ 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] = (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 package( @@ -138,60 +106,32 @@ def package( 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 is not None and not self._ignore_repository_names: - return self.repository(repository).package(name, version, extras=extras) + if repository_name and not self._ignore_repository_names: + return self.repository(repository_name).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/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 9d660ad7b46..dc68bf0015b 100644 --- a/tests/repositories/test_pool.py +++ b/tests/repositories/test_pool.py @@ -32,7 +32,7 @@ def test_pool_with_initial_repositories() -> None: def test_repository_no_repository() -> None: pool = Pool() - with pytest.raises(ValueError): + with pytest.raises(IndexError): pool.repository("foo") @@ -178,7 +178,9 @@ def test_pool_get_package_in_specified_repository() -> None: repo2 = Repository("repo2", [package]) pool = Pool([repo1, repo2]) - returned_package = pool.package("foo", Version.parse("1.0.0"), repository="repo2") + returned_package = pool.package( + "foo", Version.parse("1.0.0"), repository_name="repo2" + ) assert returned_package == package @@ -198,7 +200,7 @@ def test_pool_no_package_from_specified_repository_raises_package_not_found() -> pool = Pool([repo1, repo2]) with pytest.raises(PackageNotFound): - pool.package("foo", Version.parse("1.0.0"), repository="repo1") + pool.package("foo", Version.parse("1.0.0"), repository_name="repo1") def test_pool_find_packages_in_any_repository() -> None: 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(): From 26c79480518594104204aaf2cfa9b3bca9a9aa36 Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Sat, 1 Oct 2022 12:23:06 +0200 Subject: [PATCH 4/6] refactor: introduce AbstractRepository --- .../repositories/abstract_repository.py | 37 +++++++++++++++++++ src/poetry/repositories/cached.py | 2 +- src/poetry/repositories/http.py | 3 +- src/poetry/repositories/pool.py | 9 ++--- src/poetry/repositories/repository.py | 9 ++--- 5 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 src/poetry/repositories/abstract_repository.py diff --git a/src/poetry/repositories/abstract_repository.py b/src/poetry/repositories/abstract_repository.py new file mode 100644 index 00000000000..6725aca279c --- /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.packages.dependency import Dependency + from poetry.core.packages.package import Package + from poetry.core.semver.version import Version + + +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/pool.py b/src/poetry/repositories/pool.py index 419d8990c2d..32340329bdc 100644 --- a/src/poetry/repositories/pool.py +++ b/src/poetry/repositories/pool.py @@ -6,6 +6,7 @@ from enum import IntEnum from typing import TYPE_CHECKING +from poetry.repositories.abstract_repository import AbstractRepository from poetry.repositories.exceptions import PackageNotFound @@ -25,13 +26,13 @@ class Priority(IntEnum): SECONDARY = enum.auto() -class Pool: +class Pool(AbstractRepository): def __init__( self, repositories: list[Repository] | None = None, ignore_repository_names: bool = False, ) -> None: - self._name = "poetry-pool" + super().__init__("poetry-pool") self._repositories: OrderedDict[ str, tuple[Repository, RepositoryPriority] ] = OrderedDict() @@ -42,10 +43,6 @@ def __init__( for repository in repositories: self.add_repository(repository) - @property - def name(self) -> str: - return self._name - @property def repositories(self) -> list[Repository]: unsorted_repositories = self._repositories.values() 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 From d1bbb4483e417c2e473919063c49f5a39547423f Mon Sep 17 00:00:00 2001 From: Bart Kamphorst Date: Mon, 3 Oct 2022 22:51:45 +0200 Subject: [PATCH 5/6] refactor: introduce PrioritizedRepository --- src/poetry/repositories/pool.py | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/src/poetry/repositories/pool.py b/src/poetry/repositories/pool.py index 32340329bdc..8a345b16cfc 100644 --- a/src/poetry/repositories/pool.py +++ b/src/poetry/repositories/pool.py @@ -3,6 +3,7 @@ import enum from collections import OrderedDict +from dataclasses import dataclass from enum import IntEnum from typing import TYPE_CHECKING @@ -26,6 +27,12 @@ class Priority(IntEnum): SECONDARY = enum.auto() +@dataclass(frozen=True) +class PrioritizedRepository: + repository: Repository + priority: Priority + + class Pool(AbstractRepository): def __init__( self, @@ -33,9 +40,7 @@ def __init__( ignore_repository_names: bool = False, ) -> None: super().__init__("poetry-pool") - self._repositories: OrderedDict[ - str, tuple[Repository, RepositoryPriority] - ] = OrderedDict() + self._repositories: OrderedDict[str, PrioritizedRepository] = OrderedDict() self._ignore_repository_names = ignore_repository_names if repositories is None: @@ -46,8 +51,10 @@ def __init__( @property def repositories(self) -> list[Repository]: unsorted_repositories = self._repositories.values() - sorted_repositories = sorted(unsorted_repositories, key=lambda p: p[1].value) - return [repo for (repo, _) in sorted_repositories] + 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._contains_priority(Priority.DEFAULT) @@ -57,7 +64,7 @@ def has_primary_repositories(self) -> bool: def _contains_priority(self, priority: Priority) -> bool: return any( - repo_prio is priority for _, repo_prio in self._repositories.values() + prio_repo.priority is priority for prio_repo in self._repositories.values() ) def has_repository(self, name: str) -> bool: @@ -66,7 +73,7 @@ def has_repository(self, name: str) -> bool: def repository(self, name: str) -> Repository: name = name.lower() if self.has_repository(name): - return self._repositories[name][0] + return self._repositories[name].repository raise IndexError(f'Repository "{name}" does not exist.') def add_repository( @@ -89,7 +96,9 @@ def add_repository( priority = Priority.DEFAULT elif secondary: priority = Priority.SECONDARY - self._repositories[repository_name] = (repository, priority) + self._repositories[repository_name] = PrioritizedRepository( + repository, priority + ) return self def remove_repository(self, name: str) -> Pool: From 796d172bedb9696d62c9657c7e04c9dc1b8aa35d Mon Sep 17 00:00:00 2001 From: Bjorn Neergaard Date: Sun, 9 Oct 2022 20:07:57 -0600 Subject: [PATCH 6/6] chore: adjust import paths for poetry-core deprecations --- src/poetry/repositories/abstract_repository.py | 2 +- tests/repositories/test_repository.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/poetry/repositories/abstract_repository.py b/src/poetry/repositories/abstract_repository.py index 6725aca279c..dbbc6a95026 100644 --- a/src/poetry/repositories/abstract_repository.py +++ b/src/poetry/repositories/abstract_repository.py @@ -6,9 +6,9 @@ if TYPE_CHECKING: + from poetry.core.constraints.version import Version from poetry.core.packages.dependency import Dependency from poetry.core.packages.package import Package - from poetry.core.semver.version import Version class AbstractRepository(ABC): diff --git a/tests/repositories/test_repository.py b/tests/repositories/test_repository.py index 83a7287cce4..551691d9ff7 100644 --- a/tests/repositories/test_repository.py +++ b/tests/repositories/test_repository.py @@ -2,7 +2,7 @@ import pytest -from poetry.core.semver.version import Version +from poetry.core.constraints.version import Version from poetry.factory import Factory from poetry.repositories import Repository