fix(contacts): address PR #876 round-2 nits

- Drop the redundant `.rstrip("/")` in `_list_object_names`; the
  `endswith("/")` guard already excludes the collection entry.
- Remove the now-unused `_get_raw_vcard` (update_contact resolves the name
  itself and calls `_fetch_raw_vcard` directly). Its only remaining caller —
  the create→read integration test — now calls `_fetch_raw_vcard` with the
  deterministic `<uid>.vcf`, saving a redundant PROPFIND.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
Chris Coutinho
2026-06-08 01:52:54 +02:00
co-authored by Claude Opus 4.8
parent 011356cca2
commit 746abba18c
2 changed files with 13 additions and 14 deletions
+9 -12
View File
@@ -249,10 +249,11 @@ class ContactsClient(BaseNextcloudClient):
if href is None or not href.text: if href is None or not href.text:
continue continue
# The collection itself is reported with a trailing slash; skip it # The collection itself is reported with a trailing slash; skip it
# so only contact objects remain. # so only contact objects remain. The guard means href.text never
# ends with "/" below, so the bare split is sufficient.
if href.text.endswith("/"): if href.text.endswith("/"):
continue continue
names.append(href.text.rstrip("/").split("/")[-1]) names.append(href.text.split("/")[-1])
return names return names
async def _resolve_object_name(self, addressbook: str, uid: str) -> str | None: async def _resolve_object_name(self, addressbook: str, uid: str) -> str | None:
@@ -578,19 +579,15 @@ class ContactsClient(BaseNextcloudClient):
logger.debug("Found %s contacts", len(contacts)) logger.debug("Found %s contacts", len(contacts))
return contacts return contacts
async def _get_raw_vcard(self, addressbook: str, uid: str) -> tuple[str, str]:
"""Get raw vCard content for a contact without parsing.
Resolves the real object filename first (it may not be ``<uid>.vcf`` —
issue #874) before fetching.
"""
object_name = await self._resolve_object_name(addressbook, uid) or f"{uid}.vcf"
return await self._fetch_raw_vcard(addressbook, object_name)
async def _fetch_raw_vcard( async def _fetch_raw_vcard(
self, addressbook: str, object_name: str self, addressbook: str, object_name: str
) -> tuple[str, str]: ) -> tuple[str, str]:
"""Fetch raw vCard content + etag for an already-resolved object name.""" """Fetch raw vCard content + etag for an already-resolved object name.
Callers that only have a surfaced ``uid`` (not the real object name)
must resolve it first via ``_resolve_object_name`` — see issue #874.
``update_contact`` does exactly that and passes the resolved name here.
"""
carddav_path = self._get_carddav_base_path() carddav_path = self._get_carddav_base_path()
url = f"{carddav_path}/{addressbook}/{object_name}" url = f"{carddav_path}/{addressbook}/{object_name}"
@@ -167,8 +167,10 @@ async def test_create_contact_persists_all_documented_fields(
contact_data=contact_data, contact_data=contact_data,
) )
try: try:
raw_vcard, _etag = await nc_client.contacts._get_raw_vcard( # create_contact always writes <uid>.vcf, so fetch that object directly
addressbook_name, contact_uid # (no PROPFIND resolution needed for a contact we just created).
raw_vcard, _etag = await nc_client.contacts._fetch_raw_vcard(
addressbook_name, f"{contact_uid}.vcf"
) )
assert "FN:Full Field User" in raw_vcard assert "FN:Full Field User" in raw_vcard
assert "EMAIL" in raw_vcard and "full@example.com" in raw_vcard assert "EMAIL" in raw_vcard and "full@example.com" in raw_vcard