Skip to content

Commit 1462323

Browse files
committed
fix: simplify resource URIs by removing query parameters due to FastMCP limitations
Removed query parameters from resource URIs and simplified to basic operations only. Users should use tools for advanced queries with parameters. Fixes #4
1 parent c781f82 commit 1462323

File tree

3 files changed

+279
-51
lines changed

3 files changed

+279
-51
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ All notable changes to this project will be documented in this file.
55
The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
66
and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
77

8+
## [Unreleased]
9+
10+
### Changed
11+
- **Resource Simplification**: Removed query parameters from resource URIs due to FastMCP limitations - use tools for advanced queries (#4)
12+
813
## [0.1.2] - 2025-06-19
914

1015
### Added

mcp_server_odoo/resources.py

Lines changed: 15 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -84,63 +84,27 @@ async def get_record(model: str, record_id: str) -> str:
8484
"""
8585
return await self._handle_record_retrieval(model, record_id)
8686

87-
# Register search resource
88-
@self.app.resource(
89-
"odoo://{model}/search?domain={domain}&fields={fields}&limit={limit}&offset={offset}&order={order}"
90-
)
91-
async def search_records(
92-
model: str,
93-
domain: Optional[str] = None,
94-
fields: Optional[str] = None,
95-
limit: Optional[str] = None,
96-
offset: Optional[str] = None,
97-
order: Optional[str] = None,
98-
) -> str:
99-
"""Search for records using domain filters.
100-
101-
Args:
102-
model: The Odoo model name (e.g., 'res.partner')
103-
domain: URL-encoded domain filter (e.g., "[['is_company','=',true]]")
104-
fields: Comma-separated list of fields to return
105-
limit: Maximum number of records to return
106-
offset: Number of records to skip (for pagination)
107-
order: Sort order (e.g., "name asc, id desc")
87+
# Register search resource (no parameters due to FastMCP limitations)
88+
@self.app.resource("odoo://{model}/search")
89+
async def search_records(model: str) -> str:
90+
"""Search records with default settings.
10891
109-
Returns:
110-
Formatted search results with pagination metadata
92+
Returns first 10 records with all fields.
93+
For more control, use the search_records tool instead.
11194
"""
112-
# Convert string parameters to proper types
113-
limit_int = int(limit) if limit else None
114-
offset_int = int(offset) if offset else None
115-
return await self._handle_search(model, domain, fields, limit_int, offset_int, order)
95+
return await self._handle_search(model, None, None, None, None, None)
11696

117-
# Register browse resource
118-
@self.app.resource("odoo://{model}/browse?ids={ids}")
119-
async def browse_records(model: str, ids: str) -> str:
120-
"""Retrieve multiple records by their IDs.
97+
# Note: Browse resource removed due to FastMCP query parameter limitations
98+
# Use get_record multiple times or search_records tool instead
12199

122-
Args:
123-
model: The Odoo model name (e.g., 'res.partner')
124-
ids: Comma-separated list of record IDs (e.g., "1,2,3,4")
100+
# Register count resource (no parameters due to FastMCP limitations)
101+
@self.app.resource("odoo://{model}/count")
102+
async def count_records(model: str) -> str:
103+
"""Count all records in the model.
125104
126-
Returns:
127-
Formatted multiple record data
128-
"""
129-
return await self._handle_browse(model, ids)
130-
131-
# Register count resource
132-
@self.app.resource("odoo://{model}/count?domain={domain}")
133-
async def count_records(model: str, domain: Optional[str] = None) -> str:
134-
"""Count records matching a domain filter.
135-
136-
Args:
137-
model: The Odoo model name (e.g., 'res.partner')
138-
domain: URL-encoded domain filter (optional)
139-
140-
Returns:
141-
Count of matching records
105+
For filtered counts, use the search_records tool with limit=0.
142106
"""
143-
return await self._handle_count(model, domain)
107+
return await self._handle_count(model, None)
144108

145109
# Register fields resource
146110
@self.app.resource("odoo://{model}/fields")
Lines changed: 259 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,259 @@
1+
"""Tests for resource URI query parameter handling.
2+
3+
This test file specifically tests the fix for issue #4 where
4+
resource URIs with query parameters were failing with "Unknown operation" errors.
5+
"""
6+
7+
import json
8+
from unittest.mock import Mock
9+
from urllib.parse import quote
10+
11+
import pytest
12+
from mcp.server.fastmcp import FastMCP
13+
14+
from mcp_server_odoo.access_control import AccessController
15+
from mcp_server_odoo.config import OdooConfig
16+
from mcp_server_odoo.odoo_connection import OdooConnection
17+
from mcp_server_odoo.resources import OdooResourceHandler
18+
19+
20+
@pytest.fixture
21+
def mock_config():
22+
"""Create a mock configuration."""
23+
config = Mock(spec=OdooConfig)
24+
config.default_limit = 10
25+
config.max_limit = 100
26+
return config
27+
28+
29+
@pytest.fixture
30+
def mock_connection():
31+
"""Create a mock Odoo connection."""
32+
conn = Mock(spec=OdooConnection)
33+
conn.is_authenticated = True
34+
return conn
35+
36+
37+
@pytest.fixture
38+
def mock_access_controller():
39+
"""Create a mock access controller."""
40+
controller = Mock(spec=AccessController)
41+
controller.validate_model_access.return_value = None
42+
return controller
43+
44+
45+
@pytest.fixture
46+
def fastmcp_app():
47+
"""Create a real FastMCP app instance."""
48+
return FastMCP(name="test-odoo-mcp")
49+
50+
51+
@pytest.fixture
52+
def resource_handler(fastmcp_app, mock_connection, mock_access_controller, mock_config):
53+
"""Create a resource handler instance with real FastMCP app."""
54+
return OdooResourceHandler(fastmcp_app, mock_connection, mock_access_controller, mock_config)
55+
56+
57+
class TestResourceQueryParameterHandling:
58+
"""Test that resource URIs with various query parameter combinations work correctly."""
59+
60+
@pytest.mark.asyncio
61+
async def test_search_with_limit_only(self, resource_handler, mock_connection):
62+
"""Test search resource with only limit parameter (issue #4 case)."""
63+
# Setup mocks
64+
mock_connection.search_count.return_value = 10
65+
mock_connection.search.return_value = [1, 2]
66+
mock_connection.read.return_value = [
67+
{"id": 1, "name": "Record 1"},
68+
{"id": 2, "name": "Record 2"},
69+
]
70+
mock_connection.fields_get.return_value = {}
71+
72+
# This should work now with the fix
73+
result = await resource_handler._handle_search("res.partner", None, None, 2, None, None)
74+
75+
# Verify the search was called with correct limit
76+
mock_connection.search.assert_called_once_with(
77+
"res.partner", [], limit=2, offset=0, order=None
78+
)
79+
80+
# Verify result contains the records
81+
assert "Record 1" in result
82+
assert "Record 2" in result
83+
assert "Showing records 1-2 of 10" in result
84+
85+
@pytest.mark.asyncio
86+
async def test_search_with_domain_only(self, resource_handler, mock_connection):
87+
"""Test search resource with only domain parameter."""
88+
# Setup domain
89+
domain = [["is_company", "=", True]]
90+
domain_encoded = quote(json.dumps(domain))
91+
92+
# Setup mocks
93+
mock_connection.search_count.return_value = 3
94+
mock_connection.search.return_value = [1, 2, 3]
95+
mock_connection.read.return_value = [
96+
{"id": 1, "name": "Company A"},
97+
{"id": 2, "name": "Company B"},
98+
{"id": 3, "name": "Company C"},
99+
]
100+
mock_connection.fields_get.return_value = {}
101+
102+
# Test search with domain only
103+
result = await resource_handler._handle_search(
104+
"res.partner", domain_encoded, None, None, None, None
105+
)
106+
107+
# Verify domain was parsed and used
108+
mock_connection.search_count.assert_called_once_with("res.partner", domain)
109+
mock_connection.search.assert_called_once_with(
110+
"res.partner", domain, limit=10, offset=0, order=None
111+
)
112+
113+
assert "Company A" in result
114+
assert "is_company = True" in result
115+
116+
@pytest.mark.asyncio
117+
async def test_search_with_fields_only(self, resource_handler, mock_connection):
118+
"""Test search resource with only fields parameter."""
119+
fields = "name,email"
120+
121+
# Setup mocks
122+
mock_connection.search_count.return_value = 1
123+
mock_connection.search.return_value = [1]
124+
mock_connection.read.return_value = [
125+
{"id": 1, "name": "Test Partner", "email": "[email protected]"}
126+
]
127+
mock_connection.fields_get.return_value = {}
128+
129+
# Test search with fields only
130+
result = await resource_handler._handle_search(
131+
"res.partner", None, fields, None, None, None
132+
)
133+
134+
# Verify fields were parsed and used
135+
mock_connection.read.assert_called_once_with("res.partner", [1], ["name", "email"])
136+
137+
assert "Fields: name, email" in result
138+
assert "Test Partner" in result
139+
assert "[email protected]" in result
140+
141+
@pytest.mark.asyncio
142+
async def test_search_with_pagination_only(self, resource_handler, mock_connection):
143+
"""Test search resource with limit and offset parameters."""
144+
# Setup mocks
145+
mock_connection.search_count.return_value = 100
146+
mock_connection.search.return_value = [21, 22, 23, 24, 25]
147+
mock_connection.read.return_value = [
148+
{"id": i, "name": f"Record {i}"} for i in range(21, 26)
149+
]
150+
mock_connection.fields_get.return_value = {}
151+
152+
# Test with pagination
153+
result = await resource_handler._handle_search("res.partner", None, None, 5, 20, None)
154+
155+
# Verify pagination
156+
mock_connection.search.assert_called_once_with(
157+
"res.partner", [], limit=5, offset=20, order=None
158+
)
159+
160+
assert "Page 5 of 20" in result # offset 20, limit 5 = page 5
161+
assert "Showing records 21-25 of 100" in result
162+
assert "Record 21" in result
163+
assert "Record 25" in result
164+
165+
@pytest.mark.asyncio
166+
async def test_search_with_domain_and_limit(self, resource_handler, mock_connection):
167+
"""Test search resource with domain and limit parameters."""
168+
domain = [["active", "=", True]]
169+
domain_encoded = quote(json.dumps(domain))
170+
171+
# Setup mocks
172+
mock_connection.search_count.return_value = 50
173+
mock_connection.search.return_value = [1, 2, 3]
174+
mock_connection.read.return_value = [
175+
{"id": i, "name": f"Active Record {i}"} for i in range(1, 4)
176+
]
177+
mock_connection.fields_get.return_value = {}
178+
179+
# Test with domain and limit
180+
result = await resource_handler._handle_search(
181+
"res.partner", domain_encoded, None, 3, None, None
182+
)
183+
184+
# Verify both domain and limit were used
185+
mock_connection.search_count.assert_called_once_with("res.partner", domain)
186+
mock_connection.search.assert_called_once_with(
187+
"res.partner", domain, limit=3, offset=0, order=None
188+
)
189+
190+
assert "active = True" in result
191+
assert "Active Record 1" in result
192+
assert "Showing records 1-3 of 50" in result
193+
194+
# Browse test removed - browse resource not supported due to FastMCP query parameter limitations
195+
# Use get_record multiple times or search_records tool instead
196+
197+
@pytest.mark.asyncio
198+
async def test_count_without_domain(self, resource_handler, mock_connection):
199+
"""Test count resource without domain parameter."""
200+
# Setup mocks
201+
mock_connection.search_count.return_value = 150
202+
203+
# Test count without domain
204+
result = await resource_handler._handle_count("res.partner", None)
205+
206+
# Verify count was called with empty domain
207+
mock_connection.search_count.assert_called_once_with("res.partner", [])
208+
209+
assert "Total count: 150 record(s)" in result
210+
assert "Search criteria: All records" in result
211+
212+
@pytest.mark.asyncio
213+
async def test_count_with_domain(self, resource_handler, mock_connection):
214+
"""Test count resource with domain parameter."""
215+
domain = [["customer_rank", ">", 0]]
216+
domain_encoded = quote(json.dumps(domain))
217+
218+
# Setup mocks
219+
mock_connection.search_count.return_value = 75
220+
221+
# Test count with domain
222+
result = await resource_handler._handle_count("res.partner", domain_encoded)
223+
224+
# Verify count was called with parsed domain
225+
mock_connection.search_count.assert_called_once_with("res.partner", domain)
226+
227+
assert "Total count: 75 record(s)" in result
228+
assert "customer_rank > 0" in result
229+
230+
231+
class TestResourceRegistration:
232+
"""Test that resources are properly registered with FastMCP."""
233+
234+
def test_search_resources_registered(self, resource_handler, fastmcp_app):
235+
"""Verify that multiple search resource patterns are registered."""
236+
# Get registered handlers from the app
237+
# Note: FastMCP doesn't expose registered resources directly,
238+
# but we can verify they work by checking the handler exists
239+
240+
# Due to FastMCP limitations, only these patterns are registered:
241+
# - odoo://{model}/search (no parameters)
242+
# - odoo://{model}/record/{record_id}
243+
# - odoo://{model}/count (no parameters)
244+
# - odoo://{model}/fields
245+
246+
# Since we can't directly inspect FastMCP's internal registry,
247+
# we verify that the handler methods exist
248+
assert hasattr(resource_handler, "_handle_search")
249+
# Browse resource removed due to FastMCP limitations
250+
assert hasattr(resource_handler, "_handle_count")
251+
assert hasattr(resource_handler, "_handle_fields")
252+
assert hasattr(resource_handler, "_handle_record_retrieval")
253+
254+
def test_count_resources_registered(self, resource_handler, fastmcp_app):
255+
"""Verify that count resource patterns are registered."""
256+
# Count only supports basic pattern due to FastMCP limitations:
257+
# - odoo://{model}/count (no parameters)
258+
259+
assert hasattr(resource_handler, "_handle_count")

0 commit comments

Comments
 (0)