test(talk): cover include_status + malformed-header paths; drop Content-Type from default headers
Addresses the missing-test and Content-Type points from the latest PR #741 review: - client/talk.py _talk_headers(): drop the manual `Content-Type: application/json`. httpx sets it automatically on requests that pass `json=`, and we no longer leak it onto bodyless GETs and DELETEs. - tests/client/talk/test_talk_api.py: - new `test_talk_list_participants_with_include_status` asserting `includeStatus=true` is forwarded. - new `test_talk_get_messages_invalid_last_given_header` covering the defensive try/except around the `X-Chat-Last-Given` parse — asserts the fallback `last_given=None` and that a warning is logged. - existing `test_talk_list_participants` extended to assert that `includeStatus` is *absent* by default. Unit tests: 13 → 15. Integration tests still 7/7. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.7
parent
8f92a7ea29
commit
9614c0b361
@@ -33,11 +33,16 @@ class TalkClient(BaseNextcloudClient):
|
|||||||
_CHAT_BASE = "/ocs/v2.php/apps/spreed/api/v1/chat"
|
_CHAT_BASE = "/ocs/v2.php/apps/spreed/api/v1/chat"
|
||||||
|
|
||||||
def _talk_headers(self) -> dict[str, str]:
|
def _talk_headers(self) -> dict[str, str]:
|
||||||
"""Standard OCS+JSON headers for spreed API calls."""
|
"""Standard OCS+JSON headers for spreed API calls.
|
||||||
|
|
||||||
|
``Content-Type`` is intentionally omitted — httpx adds it
|
||||||
|
automatically (and correctly) on requests that pass ``json=``,
|
||||||
|
so setting it here would also leak it onto bodyless GETs and
|
||||||
|
DELETEs.
|
||||||
|
"""
|
||||||
return {
|
return {
|
||||||
"OCS-APIRequest": "true",
|
"OCS-APIRequest": "true",
|
||||||
"Accept": "application/json",
|
"Accept": "application/json",
|
||||||
"Content-Type": "application/json",
|
|
||||||
}
|
}
|
||||||
|
|
||||||
# Conversations (rooms)
|
# Conversations (rooms)
|
||||||
|
|||||||
@@ -247,6 +247,26 @@ async def test_talk_get_messages_pagination_cursor(mocker):
|
|||||||
assert params["includeLastKnown"] == 1
|
assert params["includeLastKnown"] == 1
|
||||||
|
|
||||||
|
|
||||||
|
async def test_talk_get_messages_invalid_last_given_header(mocker, caplog):
|
||||||
|
"""A non-numeric X-Chat-Last-Given falls back to None and logs a warning."""
|
||||||
|
mock_response = create_mock_response(
|
||||||
|
status_code=200,
|
||||||
|
headers={"X-Chat-Last-Given": "not-a-number"},
|
||||||
|
json_data={"ocs": {"meta": {"status": "ok"}, "data": []}},
|
||||||
|
)
|
||||||
|
mocker.patch.object(TalkClient, "_make_request", return_value=mock_response)
|
||||||
|
|
||||||
|
client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser")
|
||||||
|
with caplog.at_level(logging.WARNING, logger="nextcloud_mcp_server.client.talk"):
|
||||||
|
messages, last_given = await client.get_messages("abc")
|
||||||
|
|
||||||
|
assert messages == []
|
||||||
|
assert last_given is None
|
||||||
|
assert any(
|
||||||
|
"Invalid X-Chat-Last-Given" in record.message for record in caplog.records
|
||||||
|
), "Expected a warning log for the malformed header"
|
||||||
|
|
||||||
|
|
||||||
async def test_talk_send_message(mocker):
|
async def test_talk_send_message(mocker):
|
||||||
"""send_message posts the message text and parses the response."""
|
"""send_message posts the message text and parses the response."""
|
||||||
mock_response = create_mock_talk_message_response(
|
mock_response = create_mock_talk_message_response(
|
||||||
@@ -384,3 +404,22 @@ async def test_talk_list_participants(mocker):
|
|||||||
call_args = mock_make_request.call_args
|
call_args = mock_make_request.call_args
|
||||||
assert call_args[0][0] == "GET"
|
assert call_args[0][0] == "GET"
|
||||||
assert "/api/v4/room/abc/participants" in call_args[0][1]
|
assert "/api/v4/room/abc/participants" in call_args[0][1]
|
||||||
|
# Default: include_status off → no includeStatus param
|
||||||
|
assert "includeStatus" not in call_args[1].get("params", {})
|
||||||
|
|
||||||
|
|
||||||
|
async def test_talk_list_participants_with_include_status(mocker):
|
||||||
|
"""include_status=True forwards includeStatus=true as a query param."""
|
||||||
|
mock_response = create_mock_response(
|
||||||
|
status_code=200,
|
||||||
|
json_data={"ocs": {"meta": {"status": "ok"}, "data": []}},
|
||||||
|
)
|
||||||
|
mock_make_request = mocker.patch.object(
|
||||||
|
TalkClient, "_make_request", return_value=mock_response
|
||||||
|
)
|
||||||
|
|
||||||
|
client = TalkClient(mocker.AsyncMock(spec=httpx.AsyncClient), "testuser")
|
||||||
|
await client.list_participants("abc", include_status=True)
|
||||||
|
|
||||||
|
params = mock_make_request.call_args[1]["params"]
|
||||||
|
assert params["includeStatus"] == "true"
|
||||||
|
|||||||
Reference in New Issue
Block a user