Ensure repository deletion is consistent (#6214)

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>

src/poetry/repositories/pool.py

@@ -112,6 +112,20 @@ class Pool(Repository):
         idx = self._lookup.get(repository_name)
         idx = self._lookup.get(repository_name)
         if idx is not None:
         if idx is not None:
             del self._repositories[idx]
             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
         return self
 
 

tests/repositories/test_pool.py

@@ -74,3 +74,71 @@ def test_repository_with_normal_default_and_secondary_repositories():
     assert pool.repository("foo") is repo1
     assert pool.repository("foo") is repo1
     assert pool.repository("bar") is repo2
     assert pool.repository("bar") is repo2
     assert pool.has_default()
     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)