perf(notes): Improve notes search performance using async iterators
This commit is contained in:
+1
-1
@@ -6,4 +6,4 @@ __pycache__/
|
|||||||
.env.*.local
|
.env.*.local
|
||||||
|
|
||||||
# Generated by pytest used to login users
|
# Generated by pytest used to login users
|
||||||
.nextcloud_oauth_shared_test_client.json
|
.nextcloud_oauth_*.json
|
||||||
|
|||||||
@@ -151,7 +151,7 @@ Each Nextcloud app has a corresponding server module that:
|
|||||||
|
|
||||||
### Testing Structure
|
### 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
|
- **Fixtures** in `tests/conftest.py` - Shared test setup and utilities
|
||||||
- Tests are marked with `@pytest.mark.integration` for selective running
|
- Tests are marked with `@pytest.mark.integration` for selective running
|
||||||
- **Important**: Integration tests run against live Docker containers. After making code changes:
|
- **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_addressbook` - Creates and cleans up test address books
|
||||||
- `temporary_contact` - Creates and cleans up test contacts
|
- `temporary_contact` - Creates and cleans up test contacts
|
||||||
- **Test specific functionality** after changes:
|
- **Test specific functionality** after changes:
|
||||||
- For Notes changes: `uv run pytest tests/integration/test_mcp.py -k "notes" -v`
|
- For Notes changes: `uv run pytest tests/server/test_mcp.py -k "notes" -v`
|
||||||
- For specific API changes: `uv run pytest tests/integration/test_notes_api.py -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)
|
- 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
|
- **Avoid creating standalone test scripts** - use pytest with proper fixtures instead
|
||||||
|
|
||||||
|
|||||||
@@ -296,8 +296,7 @@ See [Configuration Guide](configuration.md) for all OAuth environment variables:
|
|||||||
|
|
||||||
The integration test suite includes comprehensive OAuth testing:
|
The integration test suite includes comprehensive OAuth testing:
|
||||||
|
|
||||||
- **Automated tests** (Playwright): [`tests/integration/test_oauth_playwright.py`](../tests/integration/test_oauth_playwright.py)
|
- **Automated tests** (Playwright): [`tests/client/test_oauth_playwright.py`](../tests/client/test_oauth_playwright.py)
|
||||||
- **Interactive tests**: [`tests/integration/test_oauth_interactive.py`](../tests/integration/test_oauth_interactive.py)
|
|
||||||
- **Fixtures**: [`tests/conftest.py`](../tests/conftest.py)
|
- **Fixtures**: [`tests/conftest.py`](../tests/conftest.py)
|
||||||
|
|
||||||
Run OAuth tests:
|
Run OAuth tests:
|
||||||
@@ -306,10 +305,7 @@ Run OAuth tests:
|
|||||||
docker-compose up --build -d mcp-oauth
|
docker-compose up --build -d mcp-oauth
|
||||||
|
|
||||||
# Run automated tests
|
# Run automated 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
|
||||||
|
|
||||||
# Run interactive tests (manual login)
|
|
||||||
uv run pytest tests/integration/test_oauth_interactive.py -v
|
|
||||||
```
|
```
|
||||||
|
|
||||||
## See Also
|
## See Also
|
||||||
|
|||||||
@@ -171,7 +171,7 @@ The integration test suite validates OAuth functionality:
|
|||||||
docker-compose up --build -d mcp-oauth
|
docker-compose up --build -d mcp-oauth
|
||||||
|
|
||||||
# Run comprehensive OAuth tests
|
# 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:
|
# Tests verify:
|
||||||
# - OAuth flow completion
|
# - OAuth flow completion
|
||||||
|
|||||||
@@ -125,8 +125,8 @@ class NextcloudClient:
|
|||||||
|
|
||||||
async def notes_search_notes(self, *, query: str):
|
async def notes_search_notes(self, *, query: str):
|
||||||
"""Search notes using token-based matching with relevance ranking."""
|
"""Search notes using token-based matching with relevance ranking."""
|
||||||
all_notes = await self.notes.get_all_notes()
|
all_notes = self.notes.get_all_notes()
|
||||||
return self._notes_search.search_notes(all_notes, query)
|
return await self._notes_search.search_notes(all_notes, query)
|
||||||
|
|
||||||
def _get_webdav_base_path(self) -> str:
|
def _get_webdav_base_path(self) -> str:
|
||||||
"""Helper to get the base WebDAV path for the authenticated user."""
|
"""Helper to get the base WebDAV path for the authenticated user."""
|
||||||
|
|||||||
@@ -1,7 +1,7 @@
|
|||||||
"""Client for Nextcloud Notes app operations."""
|
"""Client for Nextcloud Notes app operations."""
|
||||||
|
|
||||||
import logging
|
import logging
|
||||||
from typing import Any, Dict, List, Optional
|
from typing import Any, AsyncIterator, Dict, Optional
|
||||||
|
|
||||||
from .base import BaseNextcloudClient
|
from .base import BaseNextcloudClient
|
||||||
|
|
||||||
@@ -16,24 +16,22 @@ class NotesClient(BaseNextcloudClient):
|
|||||||
response = await self._make_request("GET", "/apps/notes/api/v1/settings")
|
response = await self._make_request("GET", "/apps/notes/api/v1/settings")
|
||||||
return response.json()
|
return response.json()
|
||||||
|
|
||||||
async def get_all_notes(self) -> List[Dict[str, Any]]:
|
async def get_all_notes(self) -> AsyncIterator[Dict[str, Any]]:
|
||||||
"""Get all notes."""
|
"""Get all notes, yielding them one at a time."""
|
||||||
notes = []
|
|
||||||
cursor = ""
|
cursor = ""
|
||||||
|
|
||||||
while True:
|
while True:
|
||||||
response = await self._make_request(
|
response = await self._make_request(
|
||||||
"GET",
|
"GET",
|
||||||
"/apps/notes/api/v1/notes",
|
"/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:
|
if "X-Notes-Chunk-Cursor" not in response.headers:
|
||||||
break
|
break
|
||||||
cursor = response.headers["X-Notes-Chunk-Cursor"]
|
cursor = response.headers["X-Notes-Chunk-Cursor"]
|
||||||
|
|
||||||
return notes
|
|
||||||
|
|
||||||
async def get_note(self, note_id: int) -> Dict[str, Any]:
|
async def get_note(self, note_id: int) -> Dict[str, Any]:
|
||||||
"""Get a specific note by ID."""
|
"""Get a specific note by ID."""
|
||||||
response = await self._make_request(
|
response = await self._make_request(
|
||||||
|
|||||||
@@ -1,13 +1,13 @@
|
|||||||
"""Controller for notes search functionality."""
|
"""Controller for notes search functionality."""
|
||||||
|
|
||||||
from typing import Any, Dict, List
|
from typing import Any, AsyncIterable, Dict, List
|
||||||
|
|
||||||
|
|
||||||
class NotesSearchController:
|
class NotesSearchController:
|
||||||
"""Handles notes search logic and scoring."""
|
"""Handles notes search logic and scoring."""
|
||||||
|
|
||||||
def search_notes(
|
async def search_notes(
|
||||||
self, notes: List[Dict[str, Any]], query: str
|
self, notes: AsyncIterable[Dict[str, Any]], query: str
|
||||||
) -> List[Dict[str, Any]]:
|
) -> List[Dict[str, Any]]:
|
||||||
"""
|
"""
|
||||||
Search notes using token-based matching with relevance ranking.
|
Search notes using token-based matching with relevance ranking.
|
||||||
@@ -21,7 +21,7 @@ class NotesSearchController:
|
|||||||
return []
|
return []
|
||||||
|
|
||||||
# Process and score each note
|
# Process and score each note
|
||||||
for note in notes:
|
async for note in notes:
|
||||||
title_tokens, content_tokens = self._process_note_content(note)
|
title_tokens, content_tokens = self._process_note_content(note)
|
||||||
score = self._calculate_score(query_tokens, title_tokens, content_tokens)
|
score = self._calculate_score(query_tokens, title_tokens, content_tokens)
|
||||||
|
|
||||||
|
|||||||
@@ -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):
|
async def test_oauth_client_notes_list(nc_oauth_client: NextcloudClient):
|
||||||
"""Test that OAuth client can list notes."""
|
"""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)
|
assert isinstance(notes, list)
|
||||||
logger.info(f"OAuth client successfully listed {len(notes)} notes")
|
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
|
# Attempt to use a protected endpoint - should fail with 401
|
||||||
# Note: capabilities endpoint is public and doesn't require auth
|
# Note: capabilities endpoint is public and doesn't require auth
|
||||||
with pytest.raises(HTTPStatusError) as exc_info:
|
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
|
assert exc_info.value.response.status_code == 401
|
||||||
|
|
||||||
|
|||||||
@@ -27,6 +27,6 @@ async def test_oauth_client_with_playwright_flow(nc_oauth_client):
|
|||||||
logger.info("OAuth client (Playwright) successfully fetched capabilities")
|
logger.info("OAuth client (Playwright) successfully fetched capabilities")
|
||||||
|
|
||||||
# Test 2: List notes
|
# 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)
|
assert isinstance(notes, list)
|
||||||
logger.info(f"OAuth client (Playwright) successfully listed {len(notes)} notes")
|
logger.info(f"OAuth client (Playwright) successfully listed {len(notes)} notes")
|
||||||
|
|||||||
@@ -97,6 +97,7 @@ async def create_mcp_client_session(
|
|||||||
|
|
||||||
finally:
|
finally:
|
||||||
# Clean up in reverse order, ignoring task scope issues
|
# Clean up in reverse order, ignoring task scope issues
|
||||||
|
# See: https://github.com/modelcontextprotocol/python-sdk/issues/577
|
||||||
if session_context is not None:
|
if session_context is not None:
|
||||||
try:
|
try:
|
||||||
await session_context.__aexit__(None, None, None)
|
await session_context.__aexit__(None, None, None)
|
||||||
|
|||||||
Reference in New Issue
Block a user