Skip to content

Commit

Permalink
Ensure repository deletion is consistent (python-poetry#6214)
Browse files Browse the repository at this point in the history
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.

Co-authored-by: Randy Döring <30527984+radoering@users.noreply.github.com>
  • Loading branch information
2 people authored and neersighted committed Aug 25, 2022
1 parent e7f864d commit d708104
Show file tree
Hide file tree
Showing 2 changed files with 82 additions and 0 deletions.
14 changes: 14 additions & 0 deletions src/poetry/repositories/pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
68 changes: 68 additions & 0 deletions tests/repositories/test_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,71 @@ 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)

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")


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)

0 comments on commit d708104

Please sign in to comment.