From e9e6bcc60a9589ef83241f96371adc8cce0bb98f Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Thu, 7 May 2026 01:41:35 +0200 Subject: [PATCH] fix(webdav): include fileid in find_by_type SEARCH + address PR #765 review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The default property set in `search_files` omits ``, so `find_by_type` returned descendant dicts with no `file_id` — which `NextcloudClient.find_files_by_tag` then silently dropped via its dedup-by-id guard. Net effect: tag-on-folder produced zero expanded descendants in CI (single-user / nc31, nc32). Mirrors the explicit property list already used in `WebDAVClient.find_by_tag`. Also addresses three nits from the PR #765 bot review: - trim multi-paragraph docstring on `_normalise_search_result` - trim multi-line docstring on `find_files_by_tag` - match `is not None` ID-extraction pattern in the descendant loop - assert positional `mime_type` arg in the unit test Co-Authored-By: Claude Opus 4.7 (1M context) --- nextcloud_mcp_server/client/__init__.py | 54 +++------------------- nextcloud_mcp_server/client/webdav.py | 18 +++++++- tests/unit/client/test_nextcloud_client.py | 8 ++-- 3 files changed, 29 insertions(+), 51 deletions(-) diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index e8e989e1..cf49be3f 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -48,14 +48,7 @@ async def log_response(response: Response): def _normalise_search_result(item: dict) -> dict: - """Normalise a webdav.search_files item to the get_files_by_tag shape. - - ``WebDAVClient.search_files`` and ``WebDAVClient.get_files_by_tag`` both - return per-file dicts but with subtly different keys (``file_id`` vs - ``id``) and path conventions (no leading slash vs leading slash). This - helper makes a search result interchangeable with a tagged-file result - so callers (notably the vector scanner) can consume both via one shape. - """ + """Normalise a webdav.search_files item to the get_files_by_tag shape.""" path = item.get("path", "") if path and not path.startswith("/"): path = "/" + path @@ -199,44 +192,7 @@ class NextcloudClient: async def find_files_by_tag( self, tag_name: str, mime_type_filter: str | None = None ) -> list[dict]: - """Find files by system tag name, optionally filtered by MIME type. - - This method coordinates tag lookup and file retrieval via WebDAV: - 1. Look up the tag ID by name - 2. Get all entries (files and directories) with that tag via REPORT - 3. For each tagged directory, walk descendants matching ``mime_type_filter`` - via WebDAV SEARCH (``Depth: infinity``) so a tag on a folder applies - to every matching file beneath it. Mirrors the directory semantics - of :mod:`nextcloud_mcp_server.server.tag_exclusion` (issue #710). - 4. Dedupe by file id — a file directly tagged AND living under a - tagged ancestor is returned once. - - Directory expansion only runs when ``mime_type_filter`` is set: - without it, expanding a tagged folder would dump the user's entire - tree into the caller, which is almost never what the operator - wanted. - - Args: - tag_name: Name of the system tag to search for (e.g., "vector-index") - mime_type_filter: Optional MIME type filter (e.g., "application/pdf"). - When set, also enables directory expansion. - - Returns: - List of file dictionaries with WebDAV properties (path, size, content_type, etc.) - - Raises: - RuntimeError: If tag lookup or the initial file query fails. A - failure walking one tagged directory is logged and skipped — other - directly-tagged files are still returned. - - Examples: - # Find all files with "vector-index" tag (no directory expansion) - files = await nc_client.find_files_by_tag("vector-index") - - # Find only PDFs with the tag, including PDFs under any folder - # that carries the tag - pdfs = await nc_client.find_files_by_tag("vector-index", "application/pdf") - """ + """Return files carrying ``tag_name``, expanding tagged folders into matching descendants when ``mime_type_filter`` is set.""" tag = await self.webdav.get_tag_by_name(tag_name) if not tag: logger.debug("Tag %r not found, returning empty list", tag_name) @@ -290,7 +246,11 @@ class NextcloudClient: for d in descendants: if d.get("is_directory"): continue - file_id = d.get("file_id") or d.get("id") + file_id = ( + d.get("file_id") + if d.get("file_id") is not None + else d.get("id") + ) if file_id is None: continue if file_id in by_id: diff --git a/nextcloud_mcp_server/client/webdav.py b/nextcloud_mcp_server/client/webdav.py index 25d004e2..978ebd1c 100644 --- a/nextcloud_mcp_server/client/webdav.py +++ b/nextcloud_mcp_server/client/webdav.py @@ -905,8 +905,24 @@ class WebDAVClient(BaseNextcloudClient): """ + # fileid is required by callers like NextcloudClient.find_files_by_tag + # that dedupe results by id; the default property set in search_files + # omits it. + properties = [ + "displayname", + "getcontentlength", + "getcontenttype", + "getlastmodified", + "resourcetype", + "getetag", + "fileid", + ] + return await self.search_files( - scope=scope, where_conditions=where_conditions, limit=limit + scope=scope, + where_conditions=where_conditions, + properties=properties, + limit=limit, ) async def list_favorites( diff --git a/tests/unit/client/test_nextcloud_client.py b/tests/unit/client/test_nextcloud_client.py index 04b95efe..1fbd9e1f 100644 --- a/tests/unit/client/test_nextcloud_client.py +++ b/tests/unit/client/test_nextcloud_client.py @@ -172,10 +172,12 @@ class TestFindFilesByTag: for f in result: assert f["path"].startswith("/") assert f["last_modified_timestamp"] is not None - # SEARCH was scoped to the tagged folder (no leading slash). + # SEARCH was scoped to the tagged folder (no leading slash) and + # forwarded the requested MIME type as the positional first arg. client.webdav.find_by_type.assert_awaited_once() - call_kwargs = client.webdav.find_by_type.await_args.kwargs - assert call_kwargs["scope"] == "corpus" + call_args = client.webdav.find_by_type.await_args + assert call_args.args[0] == "application/pdf" + assert call_args.kwargs["scope"] == "corpus" async def test_dedupes_when_file_directly_tagged_and_under_tagged_folder(self): client = _make_client()