From 92c4bf36f68c188fff7627ba36c17ec9c87ad87f Mon Sep 17 00:00:00 2001 From: Chris Coutinho Date: Sat, 29 Nov 2025 17:22:51 +0100 Subject: [PATCH] perf(news): use direct API endpoint for get_item() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace O(n) fetch-all-and-filter approach with O(1) direct API call. The News API v1-3 supports GET /items/{id} for single-item retrieval. - Update get_item() to use direct endpoint - Add unit test for get_item() method - Fixes critical performance issue identified in code review 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude --- nextcloud_mcp_server/client/news.py | 15 +++------------ tests/client/news/test_news_api.py | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+), 12 deletions(-) diff --git a/nextcloud_mcp_server/client/news.py b/nextcloud_mcp_server/client/news.py index 679a882..5085fdc 100644 --- a/nextcloud_mcp_server/client/news.py +++ b/nextcloud_mcp_server/client/news.py @@ -228,10 +228,6 @@ class NewsClient(BaseNextcloudClient): async def get_item(self, item_id: int) -> dict[str, Any]: """Get a specific item by ID. - Note: The News API doesn't have a direct single-item endpoint, - so we fetch all items and filter. For efficiency, consider - caching or using get_items with specific feed if known. - Args: item_id: Item ID @@ -239,15 +235,10 @@ class NewsClient(BaseNextcloudClient): Item data Raises: - ValueError: If item not found + HTTPStatusError: 404 if item not found """ - # Fetch all items and find the one we need - # This is inefficient but the API doesn't provide a direct endpoint - items = await self.get_items(batch_size=-1, get_read=True) - for item in items: - if item.get("id") == item_id: - return item - raise ValueError(f"Item {item_id} not found") + response = await self._make_request("GET", f"{self.API_BASE}/items/{item_id}") + return response.json() async def get_updated_items( self, diff --git a/tests/client/news/test_news_api.py b/tests/client/news/test_news_api.py index bd9f972..091693f 100644 --- a/tests/client/news/test_news_api.py +++ b/tests/client/news/test_news_api.py @@ -309,6 +309,25 @@ async def test_news_api_get_items_unread_only(mocker): assert params["getRead"] == "false" +async def test_news_api_get_item(mocker): + """Test that get_item fetches a single item by ID.""" + item = create_mock_news_item(item_id=123, title="Single Item") + mock_response = create_mock_response(status_code=200, json_data=item) + + mock_client = mocker.AsyncMock(spec=httpx.AsyncClient) + mock_make_request = mocker.patch.object( + NewsClient, "_make_request", return_value=mock_response + ) + + client = NewsClient(mock_client, "testuser") + result = await client.get_item(item_id=123) + + assert result["id"] == 123 + assert result["title"] == "Single Item" + + mock_make_request.assert_called_once_with("GET", "/apps/news/api/v1-3/items/123") + + async def test_news_api_get_updated_items(mocker): """Test that get_updated_items correctly calls the updated endpoint.""" items = [create_mock_news_item(item_id=1)]