diff --git a/nextcloud_mcp_server/client/contacts.py b/nextcloud_mcp_server/client/contacts.py index ae0aef81..f4eb1678 100644 --- a/nextcloud_mcp_server/client/contacts.py +++ b/nextcloud_mcp_server/client/contacts.py @@ -258,7 +258,7 @@ class ContactsClient(BaseNextcloudClient): async def _resolve_object_name(self, addressbook: str, uid: str) -> str | None: """Map a surfaced contact id back to its real CardDAV object filename. - ``list_contacts`` surfaces ``vcard_id`` with any ``.vcf`` suffix + ``list_contacts`` surfaces ``vcard_id`` with a trailing ``.vcf`` suffix stripped, so reverse that transform here: return the object whose filename reduces to ``uid``. The conventional ``.vcf`` is preferred when present (deterministic for the common case), otherwise @@ -267,7 +267,7 @@ class ContactsClient(BaseNextcloudClient): candidates = [ name for name in await self._list_object_names(addressbook) - if name.replace(".vcf", "") == uid + if name.removesuffix(".vcf") == uid ] if not candidates: return None @@ -512,7 +512,9 @@ class ContactsClient(BaseNextcloudClient): continue # ``vcard_id`` keeps the historical ``.vcf``-stripped form for # backward compatibility with callers that use it as the contact id. - vcard_id = object_name.replace(".vcf", "") + # Must use the same trailing-suffix strip as ``_resolve_object_name`` + # so the surface-then-resolve round-trip stays lossless (issue #874). + vcard_id = object_name.removesuffix(".vcf") # Get properties propstat = response_elem.find(".//d:propstat", ns) diff --git a/tests/unit/client/test_contacts.py b/tests/unit/client/test_contacts.py index c95de897..d332e455 100644 --- a/tests/unit/client/test_contacts.py +++ b/tests/unit/client/test_contacts.py @@ -296,6 +296,55 @@ class TestObjectNameResolution: "/remote.php/dav/addressbooks/users/testuser/contacts/ghost.vcf", ) + async def test_update_targets_real_no_extension_path(self, mocker): + """Regression for #874: update must PUT to ``.../default`` not + ``.../default.vcf`` (parallels the delete coverage). + """ + client = ContactsClient.__new__(ContactsClient) + client.username = "testuser" + mocker.patch.object( + client, "_resolve_object_name", mocker.AsyncMock(return_value="default") + ) + mocker.patch.object( + client, + "_fetch_raw_vcard", + mocker.AsyncMock( + return_value=( + "BEGIN:VCARD\nVERSION:3.0\nUID:default\nFN:No Ext\nEND:VCARD\n", + '"etag"', + ) + ), + ) + make_request = mocker.patch.object(client, "_make_request", mocker.AsyncMock()) + await client.update_contact( + addressbook="contacts", uid="default", contact_data={"fn": "Updated"} + ) + method, url = make_request.await_args.args[0], make_request.await_args.args[1] + assert method == "PUT" + assert url == "/remote.php/dav/addressbooks/users/testuser/contacts/default" + + async def test_update_falls_back_to_vcf_when_unresolved(self, mocker): + """When resolution finds nothing, update falls back to ``.vcf`` so + the caller still gets a clean 404 from the PUT (mirrors delete). + """ + client = ContactsClient.__new__(ContactsClient) + client.username = "testuser" + mocker.patch.object( + client, "_resolve_object_name", mocker.AsyncMock(return_value=None) + ) + make_request = mocker.patch.object(client, "_make_request", mocker.AsyncMock()) + # Supplying an etag skips the existing-vCard fetch; update builds a fresh + # vCard and PUTs it to the fallback path. + await client.update_contact( + addressbook="contacts", + uid="ghost", + contact_data={"fn": "Ghost"}, + etag='"x"', + ) + method, url = make_request.await_args.args[0], make_request.await_args.args[1] + assert method == "PUT" + assert url == "/remote.php/dav/addressbooks/users/testuser/contacts/ghost.vcf" + class TestFirstCustom: """``_first_custom`` is the read-side companion to PR #719 — it pulls