From 83917b37862cf01aad7874062d4d8aa7835acefa Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 18 Oct 2025 16:52:45 +0200 Subject: [PATCH] perf(notes): Improve notes search performance using async iterators --- .gitignore | 2 +- CLAUDE.md | 6 +++--- docs/oauth-architecture.md | 8 ++------ docs/oauth-upstream-status.md | 2 +- nextcloud_mcp_server/client/__init__.py | 4 ++-- nextcloud_mcp_server/client/notes.py | 14 ++++++-------- nextcloud_mcp_server/controllers/notes_search.py | 8 ++++---- tests/client/test_oauth.py | 4 ++-- tests/client/test_oauth_playwright.py | 2 +- tests/conftest.py | 1 + 10 files changed, 23 insertions(+), 28 deletions(-) diff --git a/.gitignore b/.gitignore index 09afc21..da98098 100644 --- a/.gitignore +++ b/.gitignore @@ -6,4 +6,4 @@ __pycache__/ .env.*.local # Generated by pytest used to login users -.nextcloud_oauth_shared_test_client.json +.nextcloud_oauth_*.json diff --git a/CLAUDE.md b/CLAUDE.md index d91307b..bea9f60 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -151,7 +151,7 @@ Each Nextcloud app has a corresponding server module that: ### Testing Structure -- **Integration tests** in `tests/integration/` and `tests/client/`, `tests/server/` - Test real Nextcloud API interactions +- **Integration tests** in `tests/client/` and `tests/server/` - Test real Nextcloud API interactions - **Fixtures** in `tests/conftest.py` - Shared test setup and utilities - Tests are marked with `@pytest.mark.integration` for selective running - **Important**: Integration tests run against live Docker containers. After making code changes: @@ -173,8 +173,8 @@ Each Nextcloud app has a corresponding server module that: - `temporary_addressbook` - Creates and cleans up test address books - `temporary_contact` - Creates and cleans up test contacts - **Test specific functionality** after changes: - - For Notes changes: `uv run pytest tests/integration/test_mcp.py -k "notes" -v` - - For specific API changes: `uv run pytest tests/integration/test_notes_api.py -v` + - For Notes changes: `uv run pytest tests/server/test_mcp.py -k "notes" -v` + - For specific API changes: `uv run pytest tests/client/notes/test_notes_api.py -v` - For OAuth changes: `uv run pytest tests/server/test_oauth*.py -v` (remember to rebuild `mcp-oauth` container) - **Avoid creating standalone test scripts** - use pytest with proper fixtures instead diff --git a/docs/oauth-architecture.md b/docs/oauth-architecture.md index cec3faa..4d44406 100644 --- a/docs/oauth-architecture.md +++ b/docs/oauth-architecture.md @@ -296,8 +296,7 @@ See [Configuration Guide](configuration.md) for all OAuth environment variables: The integration test suite includes comprehensive OAuth testing: -- **Automated tests** (Playwright): [`tests/integration/test_oauth_playwright.py`](../tests/integration/test_oauth_playwright.py) -- **Interactive tests**: [`tests/integration/test_oauth_interactive.py`](../tests/integration/test_oauth_interactive.py) +- **Automated tests** (Playwright): [`tests/client/test_oauth_playwright.py`](../tests/client/test_oauth_playwright.py) - **Fixtures**: [`tests/conftest.py`](../tests/conftest.py) Run OAuth tests: @@ -306,10 +305,7 @@ Run OAuth tests: docker-compose up --build -d mcp-oauth # Run automated tests -uv run pytest tests/integration/test_oauth_playwright.py --browser firefox -v - -# Run interactive tests (manual login) -uv run pytest tests/integration/test_oauth_interactive.py -v +uv run pytest tests/client/test_oauth_playwright.py --browser firefox -v ``` ## See Also diff --git a/docs/oauth-upstream-status.md b/docs/oauth-upstream-status.md index bdfc593..2d9b729 100644 --- a/docs/oauth-upstream-status.md +++ b/docs/oauth-upstream-status.md @@ -171,7 +171,7 @@ The integration test suite validates OAuth functionality: docker-compose up --build -d mcp-oauth # Run comprehensive OAuth tests -uv run pytest tests/integration/test_oauth_playwright.py --browser firefox -v +uv run pytest tests/client/test_oauth_playwright.py --browser firefox -v # Tests verify: # - OAuth flow completion diff --git a/nextcloud_mcp_server/client/__init__.py b/nextcloud_mcp_server/client/__init__.py index c363c38..ae37e79 100644 --- a/nextcloud_mcp_server/client/__init__.py +++ b/nextcloud_mcp_server/client/__init__.py @@ -125,8 +125,8 @@ class NextcloudClient: async def notes_search_notes(self, *, query: str): """Search notes using token-based matching with relevance ranking.""" - all_notes = await self.notes.get_all_notes() - return self._notes_search.search_notes(all_notes, query) + all_notes = self.notes.get_all_notes() + return await self._notes_search.search_notes(all_notes, query) def _get_webdav_base_path(self) -> str: """Helper to get the base WebDAV path for the authenticated user.""" diff --git a/nextcloud_mcp_server/client/notes.py b/nextcloud_mcp_server/client/notes.py index 95deff7..754bd75 100644 --- a/nextcloud_mcp_server/client/notes.py +++ b/nextcloud_mcp_server/client/notes.py @@ -1,7 +1,7 @@ """Client for Nextcloud Notes app operations.""" import logging -from typing import Any, Dict, List, Optional +from typing import Any, AsyncIterator, Dict, Optional from .base import BaseNextcloudClient @@ -16,24 +16,22 @@ class NotesClient(BaseNextcloudClient): response = await self._make_request("GET", "/apps/notes/api/v1/settings") return response.json() - async def get_all_notes(self) -> List[Dict[str, Any]]: - """Get all notes.""" - notes = [] + async def get_all_notes(self) -> AsyncIterator[Dict[str, Any]]: + """Get all notes, yielding them one at a time.""" cursor = "" while True: response = await self._make_request( "GET", "/apps/notes/api/v1/notes", - params={"chunkSize": 50, "chunkCursor": cursor}, + params={"chunkSize": 10, "chunkCursor": cursor}, ) - notes.extend(response.json()) + for note in response.json(): + yield note if "X-Notes-Chunk-Cursor" not in response.headers: break cursor = response.headers["X-Notes-Chunk-Cursor"] - return notes - async def get_note(self, note_id: int) -> Dict[str, Any]: """Get a specific note by ID.""" response = await self._make_request( diff --git a/nextcloud_mcp_server/controllers/notes_search.py b/nextcloud_mcp_server/controllers/notes_search.py index 35f7357..7ef8edc 100644 --- a/nextcloud_mcp_server/controllers/notes_search.py +++ b/nextcloud_mcp_server/controllers/notes_search.py @@ -1,13 +1,13 @@ """Controller for notes search functionality.""" -from typing import Any, Dict, List +from typing import Any, AsyncIterable, Dict, List class NotesSearchController: """Handles notes search logic and scoring.""" - def search_notes( - self, notes: List[Dict[str, Any]], query: str + async def search_notes( + self, notes: AsyncIterable[Dict[str, Any]], query: str ) -> List[Dict[str, Any]]: """ Search notes using token-based matching with relevance ranking. @@ -21,7 +21,7 @@ class NotesSearchController: return [] # Process and score each note - for note in notes: + async for note in notes: title_tokens, content_tokens = self._process_note_content(note) score = self._calculate_score(query_tokens, title_tokens, content_tokens) diff --git a/tests/client/test_oauth.py b/tests/client/test_oauth.py index debf0f4..284f0b2 100644 --- a/tests/client/test_oauth.py +++ b/tests/client/test_oauth.py @@ -30,7 +30,7 @@ async def test_oauth_client_capabilities(nc_oauth_client: NextcloudClient): async def test_oauth_client_notes_list(nc_oauth_client: NextcloudClient): """Test that OAuth client can list notes.""" - notes = await nc_oauth_client.notes.get_all_notes() + notes = [note async for note in nc_oauth_client.notes.get_all_notes()] assert isinstance(notes, list) logger.info(f"OAuth client successfully listed {len(notes)} notes") @@ -95,7 +95,7 @@ async def test_invalid_token_fails(): # Attempt to use a protected endpoint - should fail with 401 # Note: capabilities endpoint is public and doesn't require auth with pytest.raises(HTTPStatusError) as exc_info: - await invalid_client.notes.get_all_notes() + _ = [note async for note in invalid_client.notes.get_all_notes()] assert exc_info.value.response.status_code == 401 diff --git a/tests/client/test_oauth_playwright.py b/tests/client/test_oauth_playwright.py index b127cf3..588404c 100644 --- a/tests/client/test_oauth_playwright.py +++ b/tests/client/test_oauth_playwright.py @@ -27,6 +27,6 @@ async def test_oauth_client_with_playwright_flow(nc_oauth_client): logger.info("OAuth client (Playwright) successfully fetched capabilities") # Test 2: List notes - notes = await nc_oauth_client.notes.get_all_notes() + notes = [note async for note in nc_oauth_client.notes.get_all_notes()] assert isinstance(notes, list) logger.info(f"OAuth client (Playwright) successfully listed {len(notes)} notes") diff --git a/tests/conftest.py b/tests/conftest.py index 3e898cc..f1a34dc 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -97,6 +97,7 @@ async def create_mcp_client_session( finally: # Clean up in reverse order, ignoring task scope issues + # See: https://github.com/modelcontextprotocol/python-sdk/issues/577 if session_context is not None: try: await session_context.__aexit__(None, None, None)