legacy: address review: fix docstrings, restore create() comment, add get_by_id test, refs #9556

src/borg/legacy/archives.py

@@ -2,7 +2,7 @@
 
 In Borg 1.x the list of archives is embedded directly in the manifest blob
 as a dict {name: {"id": bytes, "time": str}}.  This class manages that dict.
-Used by ``borg transfer --from-borg1`` and ``borg serve`` (serving v1 clients).
+Used by ``borg transfer --from-borg1``.
 
 This module can be removed entirely when Borg 1.x support is dropped.
 """

src/borg/manifest.py

@@ -71,7 +71,7 @@ def filter_archives_by_date(archives, older=None, newer=None, oldest=None, newes
 
 
 @runtime_checkable
-class ArchivesInterface(Protocol):
+class ArchivesInterface(Protocol):  # pragma: no cover
     """
     Structural interface that both Archives and LegacyArchives must satisfy.
 
@@ -122,7 +122,6 @@ class Archives:
     Manage the list of archives for a Borg 2.x repository.
 
     Each archive has a separate entry in borgstore at archives/<hex-id>.
-    The manifest blob itself carries an empty archives dict.
     """
 
     def __init__(self, repository, manifest):
@@ -310,7 +309,6 @@ class Archives:
                     )
                 return archive_info
         return None  # id not in store, or archive metadata blob missing from repo
-        # TODO: add a test that calls get_by_id() with a non-existent id and asserts None is returned
 
     def create(self, name, id, ts, *, overwrite=False):
         assert isinstance(name, str)
@@ -318,8 +316,7 @@ class Archives:
         if isinstance(ts, datetime):
             ts = ts.isoformat(timespec="microseconds")
         assert isinstance(ts, str)
-        # overwrite is not enforced: archive IDs are content-addressed so a genuine
-        # duplicate (same ID, different intent) cannot occur in a correct implementation.
+        # we only create a directory entry, its name points to the archive item:
         self.repository.store_store(f"archives/{bin_to_hex(id)}", b"")
 
     def delete_by_id(self, id):

src/borg/testsuite/archive_test.py

@@ -13,7 +13,7 @@ from ..archive import Archive, CacheChunkBuffer, RobustUnpacker, valid_msgpacked
 from ..archive import BackupOSError, backup_io, backup_io_iter, get_item_uid_gid
 from ..helpers import msgpack
 from ..item import Item, ArchiveItem
-from ..manifest import Manifest
+from ..manifest import Archives, Manifest
 from ..platform import uid2user, gid2group, is_win32
 
 
@@ -427,3 +427,11 @@ def test_reject_non_sanitized_item():
     for path in rejected_dotdot_paths:
         with pytest.raises(ValueError, match="unexpected '..' element in path"):
             Item(path=path, user="root", group="root")
+
+
+def test_archives_get_by_id_missing_returns_none():
+    repo = Mock()
+    repo.store_list.return_value = []  # empty store — id will not be found
+    manifest = Mock()
+    archives = Archives(repo, manifest)
+    assert archives.get_by_id(b"\x01" * 32) is None