Reject ambiguous library names and empty library-set entries (#33)

* Reject ambiguous library names and empty library-set entries

get_library_id() silently picked the first library matching a
requested name, so two libraries sharing a name would sync whichever
happened to come first in the server's response - not necessarily the
one the operator meant. It now raises ConfigError when a name matches
more than one library.

resolve_libraries() rejects an empty entry produced by an empty
LIBRARY_ID, 'a::b', or a leading/trailing ':' the same way, instead of
silently logging 'library  not found' and moving on.

* Clarify get_library_id's ambiguity docstring and error message

Copilot review: the input can be a name or an ID, and the error said
'Library name' even for an ID collision; use neutral wording. Also
made the docstring's second sentence a complete one.

---------

Co-authored-by: Maxim Shulga <maxim.shulga@alludo.com>

IMPROVEMENTS.md

@@ -185,7 +185,13 @@ Acceptance criteria:
   `seaf-cli list --json` is used instead of splitting the display columns,
   which could not represent a library name or a path containing whitespace.
   `list-remote` still uses the HTTP API rather than the CLI.
-- [ ] Reject ambiguous library names and an empty requested library set.
+- [x] Reject ambiguous library names and an empty requested library set.
+  `get_library_id` used to silently return the first library matching a
+  requested name, even if two libraries shared it; it now raises
+  `ConfigError` when a name matches more than one library ID.
+  `resolve_libraries` rejects an empty entry from `LIBRARY_ID` (an empty
+  string, or one produced by `"a::b"`/a leading or trailing `:`) the same
+  way. See `tests/test_libraries.py`.
 
 ### Runtime hardening
 

dsc/client.py

@@ -166,10 +166,22 @@ class SeafileClient:
         _lg.info("Seafile daemon is stopped")
 
     def get_library_id(self, library) -> Optional[str]:
-        for lib_id, lib_name in self.remote_libraries.items():
+        """
-            if library in (lib_id, lib_name):
+        Resolve a requested name or ID to the one library ID it names.
-                return lib_id
+
-        return None
+        Library IDs are unique, but two libraries can share a name. Silently
+        matching more than one and picking the first would sync whichever
+        happened to come first in the server's response, not necessarily the
+        one requested.
+        """
+        matches = [lib_id for lib_id, lib_name in self.remote_libraries.items()
+                   if library in (lib_id, lib_name)]
+        if len(matches) > 1:
+            raise ConfigError(
+                f"{library!r} matches {len(matches)} libraries on the server; "
+                "use the library ID instead"
+            )
+        return matches[0] if matches else None
 
     def sync_lib(self, lib_id: str, lib_dir: str) -> bool:
         """

start.py

@@ -53,8 +53,14 @@ def defaults_from_env() -> dict:
 
 def resolve_libraries(client, requested: str, server: str) -> set:
     """Map the requested names or IDs to library IDs that are not synced yet."""
+    requested_names = requested.split(sep=":")
+    if any(name == "" for name in requested_names):
+        raise ConfigError(
+            f"library set must not contain an empty entry, got {requested!r}"
+        )
+
     libs_to_sync = set()
-    for arg_lib in requested.split(sep=":"):
+    for arg_lib in requested_names:
         lib_id = client.get_library_id(arg_lib)
         if lib_id:
             libs_to_sync.add(lib_id)

tests/test_libraries.py

@@ -0,0 +1,56 @@
+"""A library set must resolve to exactly one library ID per entry, or fail."""
+
+import pytest
+
+from dsc.client import SeafileClient
+from dsc.errors import ConfigError
+from start import resolve_libraries
+
+
+@pytest.fixture
+def client(monkeypatch):
+    c = SeafileClient("seafile.example", "user", "pw", "/dsc")
+    monkeypatch.setattr(type(c), "remote_libraries", {
+        "id-1": "Docs",
+        "id-2": "Docs",
+        "id-3": "Photos",
+    })
+    monkeypatch.setattr(c, "get_local_libraries", lambda: set())
+    return c
+
+
+def test_get_library_id_resolves_a_unique_name(client):
+    assert client.get_library_id("Photos") == "id-3"
+
+
+def test_get_library_id_resolves_by_id(client):
+    assert client.get_library_id("id-1") == "id-1"
+
+
+def test_get_library_id_returns_none_for_no_match(client):
+    assert client.get_library_id("Missing") is None
+
+
+def test_get_library_id_rejects_an_ambiguous_name(client):
+    with pytest.raises(ConfigError, match="Docs.*2 libraries"):
+        client.get_library_id("Docs")
+
+
+def test_resolve_libraries_rejects_an_empty_requested_set(client):
+    with pytest.raises(ConfigError, match="empty entry"):
+        resolve_libraries(client, "", "seafile.example")
+
+
+@pytest.mark.parametrize("requested", ["Photos:", ":Photos", "Photos::id-1"])
+def test_resolve_libraries_rejects_an_empty_entry_between_separators(client, requested):
+    with pytest.raises(ConfigError, match="empty entry"):
+        resolve_libraries(client, requested, "seafile.example")
+
+
+def test_resolve_libraries_rejects_an_ambiguous_name_end_to_end(client):
+    with pytest.raises(ConfigError, match="Docs"):
+        resolve_libraries(client, "Docs", "seafile.example")
+
+
+def test_resolve_libraries_resolves_unambiguous_names(client):
+    assert resolve_libraries(client, "Photos:id-1", "seafile.example") == {"id-3", "id-1"}