From e7fbd8647921a9120e7f5a281960df5345bcb5b6 Mon Sep 17 00:00:00 2001 From: Jacob Emmert-Aronson Date: Mon, 22 Aug 2022 12:51:19 -0700 Subject: [PATCH 1/2] Ensure repository deletion is consistent This change improves the consistency of Pool.remove_repository() to make it easier to write poetry plugins which mutate the repository pool. 1. Deleting an element from the middle of Pool()._repositories decrements the index of later entries. Update Pool()._lookup to reflect this. 2. If a primary repository is deleted, decrement Pool()._secondary_start_idx to ensure that any additional primary repositories are still added before all secondary repositories. 3. If the default repository is deleted, reset Pool()._default so a new one can be added. --- src/poetry/repositories/pool.py | 14 ++++++++ tests/repositories/test_pool.py | 61 +++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/src/poetry/repositories/pool.py b/src/poetry/repositories/pool.py index b67e57052ae..c7fdcac0115 100644 --- a/src/poetry/repositories/pool.py +++ b/src/poetry/repositories/pool.py @@ -112,6 +112,20 @@ def remove_repository(self, repository_name: str) -> Pool: 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 return self diff --git a/tests/repositories/test_pool.py b/tests/repositories/test_pool.py index 8b27b96284e..9e880b1824c 100644 --- a/tests/repositories/test_pool.py +++ b/tests/repositories/test_pool.py @@ -74,3 +74,64 @@ 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() + + +def test_remove_repository(): + repo1 = LegacyRepository("foo", "https://foo.bar") + repo2 = LegacyRepository("bar", "https://bar.baz") + repo3 = LegacyRepository("baz", "https://baz.quux") + + pool = Pool() + pool.add_repository(repo1) + pool.add_repository(repo2) + pool.add_repository(repo3) + pool.remove_repository("bar") + + assert pool.repository("foo") is repo1 + assert not pool.has_repository("bar") + assert pool.repository("baz") is repo3 + + +def test_remove_default_repository(): + default = LegacyRepository("default", "https://default.com") + repo1 = LegacyRepository("foo", "https://foo.bar") + repo2 = LegacyRepository("bar", "https://bar.baz") + new_default = LegacyRepository("new_default", "https://new.default.com") + + pool = Pool() + pool.add_repository(repo1) + pool.add_repository(repo2) + pool.add_repository(default, default=True) + pool.remove_repository("default") + pool.add_repository(new_default, default=True) + + assert pool.repositories[0] is new_default + assert not pool.has_repository("default") + + +def test_repository_ordering(): + default1 = LegacyRepository("default1", "https://default1.com") + default2 = LegacyRepository("default2", "https://default2.com") + primary1 = LegacyRepository("primary1", "https://primary1.com") + primary2 = LegacyRepository("primary2", "https://primary2.com") + primary3 = LegacyRepository("primary3", "https://primary3.com") + secondary1 = LegacyRepository("secondary1", "https://secondary1.com") + secondary2 = LegacyRepository("secondary2", "https://secondary2.com") + secondary3 = LegacyRepository("secondary3", "https://secondary3.com") + + pool = Pool() + pool.add_repository(secondary1, secondary=True) + pool.add_repository(primary1) + pool.add_repository(default1, default=True) + pool.add_repository(primary2) + pool.add_repository(secondary2, secondary=True) + + pool.remove_repository("primary2") + pool.remove_repository("secondary2") + + pool.add_repository(primary3) + pool.add_repository(secondary3, secondary=True) + + assert pool.repositories == [default1, primary1, primary3, secondary1, secondary3] + with pytest.raises(ValueError): + pool.add_repository(default2, default=True) From 61e6714412ca334d2d6afee8a94ea1f1ba05ed3b Mon Sep 17 00:00:00 2001 From: Jacob Emmert-Aronson Date: Tue, 23 Aug 2022 09:13:08 -0700 Subject: [PATCH 2/2] Improve unit test coverage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com> --- tests/repositories/test_pool.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/tests/repositories/test_pool.py b/tests/repositories/test_pool.py index 9e880b1824c..3353115eac7 100644 --- a/tests/repositories/test_pool.py +++ b/tests/repositories/test_pool.py @@ -102,9 +102,16 @@ def test_remove_default_repository(): pool.add_repository(repo1) pool.add_repository(repo2) pool.add_repository(default, default=True) + + assert pool.has_default() + pool.remove_repository("default") + + assert not pool.has_default() + pool.add_repository(new_default, default=True) + assert pool.has_default() assert pool.repositories[0] is new_default assert not pool.has_repository("default")