Skip to content

Commit e375bd8

Browse files
Improved validation of response data in select ConversationsOperations methods (#317)
* Improved validation of received response data * Addressing PR comments * Revising tests and implementing suggested edits
1 parent b1adaaf commit e375bd8

3 files changed

Lines changed: 152 additions & 6 deletions

File tree

libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/connector_client.py

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -216,21 +216,29 @@ async def reply_to_activity(
216216
by_alias=True, exclude_unset=True, exclude_none=True, mode="json"
217217
),
218218
) as response:
219-
result = await response.json() if response.content_length else {}
219+
220+
response_text = await response.text("utf-8")
220221

221222
if response.status >= 300:
222223
logger.error(
223224
"Error replying to activity: %s",
224-
result or response.status,
225+
response_text or response.status,
225226
stack_info=True,
226227
)
227228
response.raise_for_status()
228229

230+
if not response_text:
231+
resource_response = ResourceResponse()
232+
else:
233+
resource_response = ResourceResponse.model_validate_json(response_text)
234+
229235
logger.info(
230-
"Reply to conversation/activity: %s, %s", result.get("id"), activity_id
236+
"Reply to conversation/activity: %s, %s",
237+
resource_response.id,
238+
activity_id,
231239
)
232240

233-
return ResourceResponse.model_validate(result)
241+
return resource_response
234242

235243
async def send_to_conversation(
236244
self, conversation_id: str, body: Activity
@@ -269,8 +277,10 @@ async def send_to_conversation(
269277
)
270278
response.raise_for_status()
271279

272-
data = await response.json()
273-
return ResourceResponse.model_validate(data)
280+
response_text = await response.text("utf-8")
281+
if not response_text:
282+
return ResourceResponse()
283+
return ResourceResponse.model_validate_json(response_text)
274284

275285
async def update_activity(
276286
self, conversation_id: str, activity_id: str, body: Activity

tests/hosting_core/connector/__init__.py

Whitespace-only changes.
Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
# Copyright (c) Microsoft Corporation. All rights reserved.
2+
# Licensed under the MIT License.
3+
4+
"""Tests for ConversationsOperations using aiohttp TestServer."""
5+
6+
import json
7+
8+
import pytest
9+
from aiohttp import web, ClientSession
10+
from aiohttp.test_utils import TestServer
11+
12+
from microsoft_agents.activity import Activity, ResourceResponse
13+
from microsoft_agents.hosting.core.connector.client.connector_client import (
14+
ConversationsOperations,
15+
)
16+
17+
18+
def _create_app(routes):
19+
"""Create an aiohttp app with the given route table."""
20+
app = web.Application()
21+
app.router.add_routes(routes)
22+
return app
23+
24+
25+
class TestSendToConversation:
26+
"""Tests for ConversationsOperations.send_to_conversation."""
27+
28+
@pytest.fixture
29+
def activity(self):
30+
return Activity(type="message", text="Hello, world!")
31+
32+
@pytest.mark.asyncio
33+
async def test_send_to_conversation_success_with_content(self, activity):
34+
"""Should return ResourceResponse validated from response text."""
35+
36+
async def handler(request):
37+
return web.json_response({"id": "activity-id-123"})
38+
39+
routes = [web.post("/v3/conversations/{conversation_id}/activities", handler)]
40+
app = _create_app(routes)
41+
42+
server = TestServer(app)
43+
await server.start_server()
44+
try:
45+
async with ClientSession(base_url=server.make_url("/")) as session:
46+
ops = ConversationsOperations(session)
47+
result = await ops.send_to_conversation("conv-1", activity)
48+
49+
assert isinstance(result, ResourceResponse)
50+
assert result.id == "activity-id-123"
51+
finally:
52+
await server.close()
53+
54+
@pytest.mark.asyncio
55+
async def test_send_to_conversation_success_no_content(self, activity):
56+
"""Should return empty ResourceResponse when no content."""
57+
58+
async def handler(request):
59+
return web.Response(status=200, text="")
60+
61+
routes = [web.post("/v3/conversations/{conversation_id}/activities", handler)]
62+
app = _create_app(routes)
63+
64+
server = TestServer(app)
65+
await server.start_server()
66+
try:
67+
async with ClientSession(base_url=server.make_url("/")) as session:
68+
ops = ConversationsOperations(session)
69+
result = await ops.send_to_conversation("conv-1", activity)
70+
71+
assert isinstance(result, ResourceResponse)
72+
assert result.id is None
73+
finally:
74+
await server.close()
75+
76+
77+
class TestReplyToActivity:
78+
"""Tests for ConversationsOperations.reply_to_activity."""
79+
80+
@pytest.fixture
81+
def activity(self):
82+
return Activity(type="message", text="Hello, world!")
83+
84+
@pytest.mark.asyncio
85+
async def test_reply_to_activity_success_with_content(self, activity):
86+
"""Should return ResourceResponse parsed from JSON response text."""
87+
88+
async def handler(request):
89+
return web.json_response({"id": "reply-id-456"})
90+
91+
routes = [
92+
web.post(
93+
"/v3/conversations/{conversation_id}/activities/{activity_id}",
94+
handler,
95+
)
96+
]
97+
app = _create_app(routes)
98+
99+
server = TestServer(app)
100+
await server.start_server()
101+
try:
102+
async with ClientSession(base_url=server.make_url("/")) as session:
103+
ops = ConversationsOperations(session)
104+
result = await ops.reply_to_activity("conv-1", "act-1", activity)
105+
106+
assert isinstance(result, ResourceResponse)
107+
assert result.id == "reply-id-456"
108+
finally:
109+
await server.close()
110+
111+
@pytest.mark.asyncio
112+
async def test_reply_to_activity_success_no_content(self, activity):
113+
"""Should return empty ResourceResponse when no content is returned."""
114+
115+
async def handler(request):
116+
return web.Response(status=200, text="")
117+
118+
routes = [
119+
web.post(
120+
"/v3/conversations/{conversation_id}/activities/{activity_id}",
121+
handler,
122+
)
123+
]
124+
app = _create_app(routes)
125+
126+
server = TestServer(app)
127+
await server.start_server()
128+
try:
129+
async with ClientSession(base_url=server.make_url("/")) as session:
130+
ops = ConversationsOperations(session)
131+
result = await ops.reply_to_activity("conv-1", "act-1", activity)
132+
133+
assert isinstance(result, ResourceResponse)
134+
assert result.id is None
135+
finally:
136+
await server.close()

0 commit comments

Comments
 (0)