fix: Address adding duplicate repositories and conflicts with manual vs package installed repositories. (#65)

This PR fixes two bugs:
1. An issue where multiple same-path repositories could be added.
2. Test cases which failed if there were package installed repositories
(in addition to manually installed ones).

Author: sdelliot

src/firewheel/control/repository_db.py

@@ -93,6 +93,10 @@ def add_repository(self, repository):
             # No database file exists yet.
             entries = []
 
+        if any(entry["path"] == repository["path"] for entry in entries):
+            self.log.debug("Ignoring duplicate repository: %s", repository)
+            return
+
         entries.append(repository)
         with self.db_file.open("w") as db:
             json.dump(entries, db)
@@ -133,7 +137,8 @@ def delete_repository(self, repository):
             return 0
         except FileNotFoundError:
             self.log.debug(
-                "%s repository path does not exist, but was removed anyways.", repository
+                "%s repository path does not exist, but was removed anyways.",
+                repository,
             )
 
         with self.db_file.open("w") as db:

src/firewheel/tests/unit/control/test_repository_db.py

@@ -37,9 +37,9 @@ class TestRepositoryDb:
     """Test the ``RepositoryDb`` object."""
 
     @staticmethod
-    def _entry_matches_repo_dict(repo_entry, repo_dict):
+    def _entry_in_repo_list(repo_entry, repo_list):
         path = repo_entry["path"]
-        return path == repo_dict["path"]
+        return any(entry["path"] == path for entry in repo_list)
 
     def test_new_repository(self, repository_db_test_path):
         location = repository_db_test_path
@@ -70,18 +70,16 @@ def test_corrupt_repository_add(self, repository_db_test_path, repo_entry):
         assert location.exists() is True
 
         repository_db.add_repository(repo_entry)
-        repo_dict = list(repository_db.list_repositories()).pop()
-        assert len(repo_dict) == 1
-        assert self._entry_matches_repo_dict(repo_entry, repo_dict)
+        repo_list = list(repository_db.list_repositories())
+        assert self._entry_in_repo_list(repo_entry, repo_list)
 
         location.unlink(missing_ok=True)
         assert location.exists() is False
 
     def test_add_repository(self, repository_db, repo_entry):
         repository_db.add_repository(repo_entry)
-        repo_dict = list(repository_db.list_repositories()).pop()
-        assert len(repo_dict) == 1
-        assert self._entry_matches_repo_dict(repo_entry, repo_dict)
+        repo_list = list(repository_db.list_repositories())
+        assert self._entry_in_repo_list(repo_entry, repo_list)
 
     @pytest.mark.parametrize(
         ["invalid_entry", "exception"],
@@ -104,11 +102,13 @@ def test_add_repository_invalid(self, invalid_entry, exception, repository_db):
             repository_db.add_repository(invalid_entry)
 
     def test_duplicate_repository(self, repository_db, repo_entry):
+        orig_entry_count = len(list(repository_db.list_repositories()))
         repository_db.add_repository(repo_entry)
         repository_db.add_repository(repo_entry)
-        repo_dict = list(repository_db.list_repositories()).pop()
-        assert len(repo_dict) == 1
-        assert self._entry_matches_repo_dict(repo_entry, repo_dict)
+        repo_list = list(repository_db.list_repositories())
+        assert self._entry_in_repo_list(repo_entry, repo_list)
+        # The entry should only be added once
+        assert len(repo_list) == orig_entry_count + 1
 
     def test_delete_repository(self, repository_db, repo_entry):
         repository_db.add_repository(repo_entry)
@@ -118,5 +118,5 @@ def test_list_repositories(self, repository_db, repo_entries):
         orig_entry_count = len(list(repository_db.list_repositories()))
         for entry in repo_entries:
             repository_db.add_repository(entry)
-        repo_list = list(repository_db.list_repositories())
-        assert len(repo_list) == orig_entry_count + len(repo_entries)
+        curr_count = len(list(repository_db.list_repositories()))
+        assert curr_count == orig_entry_count + len(repo_entries)