Skip to content

Commit 948bfb9

Browse files
Switches Fogbugz client from aiohttp to httpx
Replaces aiohttp with httpx for async HTTP requests, simplifying multipart form data handling and removing dependency on aiohttp client sessions. Updates tests to use respx for mocking httpx requests, improving test reliability and isolation.
1 parent beae0ba commit 948bfb9

File tree

2 files changed

+121
-81
lines changed

2 files changed

+121
-81
lines changed

services/web/server/src/simcore_service_webserver/fogbugz/_client.py

Lines changed: 67 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,19 @@
66
import json
77
import logging
88

9-
import aiohttp
9+
import httpx
1010
from aiohttp import web
1111
from pydantic import BaseModel, Field
12-
from servicelib.aiohttp.client_session import get_client_session
1312

1413
from ..products import products_service
1514
from ..products.models import Product
1615
from .settings import FogbugzSettings, get_plugin_settings
1716

1817
_logger = logging.getLogger(__name__)
1918

19+
_JSON_CONTENT_TYPE = "application/json"
20+
_UNKNOWN_ERROR_MESSAGE = "Unknown error occurred"
21+
2022

2123
class FogbugzCaseCreate(BaseModel):
2224
fogbugz_project_id: str = Field(description="Project ID in Fogbugz")
@@ -27,8 +29,10 @@ class FogbugzCaseCreate(BaseModel):
2729
class FogbugzRestClient:
2830
"""REST client for Fogbugz API"""
2931

30-
def __init__(self, app: web.Application, api_token: str, base_url: str) -> None:
31-
self._app = app
32+
def __init__(
33+
self, client: httpx.AsyncClient, api_token: str, base_url: str
34+
) -> None:
35+
self._client = client
3236
self._api_token = api_token
3337
self._base_url = base_url
3438

@@ -43,25 +47,21 @@ async def create_case(self, data: FogbugzCaseCreate) -> str:
4347
}
4448

4549
# Fogbugz requires multipart/form-data with stringified JSON
46-
form_data = aiohttp.FormData()
47-
form_data.add_field(
48-
"request", json.dumps(json_payload), content_type="application/json"
49-
)
50+
files = {"request": (None, json.dumps(json_payload), _JSON_CONTENT_TYPE)}
5051

51-
session = get_client_session(self._app)
5252
url = f"{self._base_url}/f/api/0/jsonapi"
5353

54-
async with session.post(url, data=form_data) as response:
55-
response.raise_for_status()
56-
response_data = await response.json()
54+
response = await self._client.post(url, files=files)
55+
response.raise_for_status()
56+
response_data = response.json()
5757

58-
# Fogbugz API returns case ID in the response
59-
case_id = response_data.get("data", {}).get("case", {}).get("ixBug", None)
60-
if case_id is None:
61-
msg = "Failed to create case in Fogbugz"
62-
raise ValueError(msg)
58+
# Fogbugz API returns case ID in the response
59+
case_id = response_data.get("data", {}).get("case", {}).get("ixBug", None)
60+
if case_id is None:
61+
msg = "Failed to create case in Fogbugz"
62+
raise ValueError(msg)
6363

64-
return str(case_id)
64+
return str(case_id)
6565

6666
async def resolve_case(self, case_id: str) -> None:
6767
"""Resolve a case in Fogbugz"""
@@ -72,23 +72,19 @@ async def resolve_case(self, case_id: str) -> None:
7272
}
7373

7474
# Fogbugz requires multipart/form-data with stringified JSON
75-
form_data = aiohttp.FormData()
76-
form_data.add_field(
77-
"request", json.dumps(json_payload), content_type="application/json"
78-
)
75+
files = {"request": (None, json.dumps(json_payload), _JSON_CONTENT_TYPE)}
7976

80-
session = get_client_session(self._app)
8177
url = f"{self._base_url}/f/api/0/jsonapi"
8278

83-
async with session.post(url, data=form_data) as response:
84-
response.raise_for_status()
85-
response_data = await response.json()
79+
response = await self._client.post(url, files=files)
80+
response.raise_for_status()
81+
response_data = response.json()
8682

87-
# Check if the operation was successful
88-
if response_data.get("error"):
89-
error_msg = response_data.get("error", "Unknown error occurred")
90-
msg = f"Failed to resolve case in Fogbugz: {error_msg}"
91-
raise ValueError(msg)
83+
# Check if the operation was successful
84+
if response_data.get("error"):
85+
error_msg = response_data.get("error", _UNKNOWN_ERROR_MESSAGE)
86+
msg = f"Failed to resolve case in Fogbugz: {error_msg}"
87+
raise ValueError(msg)
9288

9389
async def get_case_status(self, case_id: str) -> str:
9490
"""Get the status of a case in Fogbugz"""
@@ -100,48 +96,44 @@ async def get_case_status(self, case_id: str) -> str:
10096
}
10197

10298
# Fogbugz requires multipart/form-data with stringified JSON
103-
form_data = aiohttp.FormData()
104-
form_data.add_field(
105-
"request", json.dumps(json_payload), content_type="application/json"
106-
)
99+
files = {"request": (None, json.dumps(json_payload), _JSON_CONTENT_TYPE)}
107100

108-
session = get_client_session(self._app)
109101
url = f"{self._base_url}/f/api/0/jsonapi"
110102

111-
async with session.post(url, data=form_data) as response:
112-
response.raise_for_status()
113-
response_data = await response.json()
103+
response = await self._client.post(url, files=files)
104+
response.raise_for_status()
105+
response_data = response.json()
114106

115-
# Check if the operation was successful
116-
if response_data.get("error"):
117-
error_msg = response_data.get("error", "Unknown error occurred")
118-
msg = f"Failed to get case status from Fogbugz: {error_msg}"
119-
raise ValueError(msg)
107+
# Check if the operation was successful
108+
if response_data.get("error"):
109+
error_msg = response_data.get("error", _UNKNOWN_ERROR_MESSAGE)
110+
msg = f"Failed to get case status from Fogbugz: {error_msg}"
111+
raise ValueError(msg)
120112

121-
# Extract the status from the search results
122-
cases = response_data.get("data", {}).get("cases", [])
123-
if not cases:
124-
msg = f"Case {case_id} not found in Fogbugz"
125-
raise ValueError(msg)
113+
# Extract the status from the search results
114+
cases = response_data.get("data", {}).get("cases", [])
115+
if not cases:
116+
msg = f"Case {case_id} not found in Fogbugz"
117+
raise ValueError(msg)
126118

127-
# Find the case with matching ixBug
128-
target_case = None
129-
for case in cases:
130-
if str(case.get("ixBug")) == str(case_id):
131-
target_case = case
132-
break
119+
# Find the case with matching ixBug
120+
target_case = None
121+
for case in cases:
122+
if str(case.get("ixBug")) == str(case_id):
123+
target_case = case
124+
break
133125

134-
if target_case is None:
135-
msg = f"Case {case_id} not found in search results"
136-
raise ValueError(msg)
126+
if target_case is None:
127+
msg = f"Case {case_id} not found in search results"
128+
raise ValueError(msg)
137129

138-
# Get the status from the found case
139-
status = target_case.get("sStatus", "")
140-
if not status:
141-
msg = f"Status not found for case {case_id}"
142-
raise ValueError(msg)
130+
# Get the status from the found case
131+
status = target_case.get("sStatus", "")
132+
if not status:
133+
msg = f"Status not found for case {case_id}"
134+
raise ValueError(msg)
143135

144-
return status
136+
return status
145137

146138
async def reopen_case(self, case_id: str, assigned_fogbugz_person_id: str) -> None:
147139
"""Reopen a case in Fogbugz (uses reactivate for resolved cases, reopen for closed cases)"""
@@ -165,23 +157,19 @@ async def reopen_case(self, case_id: str, assigned_fogbugz_person_id: str) -> No
165157
}
166158

167159
# Fogbugz requires multipart/form-data with stringified JSON
168-
form_data = aiohttp.FormData()
169-
form_data.add_field(
170-
"request", json.dumps(json_payload), content_type="application/json"
171-
)
160+
files = {"request": (None, json.dumps(json_payload), _JSON_CONTENT_TYPE)}
172161

173-
session = get_client_session(self._app)
174162
url = f"{self._base_url}/f/api/0/jsonapi"
175163

176-
async with session.post(url, data=form_data) as response:
177-
response.raise_for_status()
178-
response_data = await response.json()
164+
response = await self._client.post(url, files=files)
165+
response.raise_for_status()
166+
response_data = response.json()
179167

180-
# Check if the operation was successful
181-
if response_data.get("error"):
182-
error_msg = response_data.get("error", "Unknown error occurred")
183-
msg = f"Failed to reopen case in Fogbugz: {error_msg}"
184-
raise ValueError(msg)
168+
# Check if the operation was successful
169+
if response_data.get("error"):
170+
error_msg = response_data.get("error", _UNKNOWN_ERROR_MESSAGE)
171+
msg = f"Failed to reopen case in Fogbugz: {error_msg}"
172+
raise ValueError(msg)
185173

186174

187175
_APP_KEY = f"{__name__}.{FogbugzRestClient.__name__}"
@@ -217,8 +205,9 @@ async def setup_fogbugz_rest_client(app: web.Application) -> None:
217205
product.name,
218206
)
219207

208+
httpx_client = httpx.AsyncClient()
220209
client = FogbugzRestClient(
221-
app=app,
210+
client=httpx_client,
222211
api_token=settings.FOGBUGZ_API_TOKEN,
223212
base_url=settings.FOGBUGZ_URL,
224213
)

services/web/server/tests/unit/with_dbs/04/test_fogbugz_client.py

Lines changed: 54 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,8 @@
1+
from collections.abc import Iterator
2+
3+
import httpx
14
import pytest
5+
import respx
26
from aiohttp.test_utils import TestClient
37
from pytest_mock import MockerFixture
48
from pytest_simcore.helpers.monkeypatch_envs import setenvs_from_dict
@@ -10,24 +14,71 @@
1014
from simcore_service_webserver.fogbugz.settings import FogbugzSettings
1115

1216

17+
@pytest.fixture
18+
def fake_api_base_url() -> str:
19+
return "https://dummy.com"
20+
21+
1322
@pytest.fixture
1423
def app_environment(
1524
monkeypatch: pytest.MonkeyPatch,
1625
app_environment: EnvVarsDict,
26+
fake_api_base_url: str,
1727
mocker: MockerFixture,
1828
):
1929
return app_environment | setenvs_from_dict(
2030
monkeypatch,
2131
{
22-
"FOGBUGZ_URL": "https://dummy.com",
23-
"FOGBUGZ_API_TOKEN": "12345",
32+
"FOGBUGZ_URL": fake_api_base_url,
33+
"FOGBUGZ_API_TOKEN": "asdf",
2434
},
2535
)
2636

2737

38+
_IXBUG_DUMMY = "12345"
39+
40+
41+
@pytest.fixture
42+
def mock_fogbugz_api(fake_api_base_url: str) -> Iterator[respx.MockRouter]:
43+
"""Mock Fogbugz API responses in sequence for the test flow"""
44+
45+
# Define responses in the order they will be called during the test
46+
responses = [
47+
# 1. create_case response
48+
{"data": {"case": {"ixBug": _IXBUG_DUMMY}}},
49+
# 2. get_case_status response (after creation)
50+
{"data": {"cases": [{"ixBug": _IXBUG_DUMMY, "sStatus": "Active"}]}},
51+
# 3. resolve_case response
52+
{"data": {}},
53+
# 4. get_case_status response (after resolve)
54+
{
55+
"data": {
56+
"cases": [{"ixBug": _IXBUG_DUMMY, "sStatus": "Resolved (Completed)"}]
57+
}
58+
},
59+
# 5. reopen_case response (inside you need to get the status ones)
60+
{
61+
"data": {
62+
"cases": [{"ixBug": _IXBUG_DUMMY, "sStatus": "Resolved (Completed)"}]
63+
}
64+
},
65+
{"data": {}},
66+
# 6. get_case_status response (after reopen)
67+
{"data": {"cases": [{"ixBug": _IXBUG_DUMMY, "sStatus": "Active"}]}},
68+
]
69+
70+
with respx.mock(base_url=fake_api_base_url) as mock:
71+
# Create a side_effect that returns responses in sequence
72+
mock.post(path="/f/api/0/jsonapi").mock(
73+
side_effect=[httpx.Response(200, json=response) for response in responses]
74+
)
75+
yield mock
76+
77+
2878
async def test_testit(
2979
app_environment: EnvVarsDict,
3080
client: TestClient,
81+
mock_fogbugz_api: respx.MockRouter,
3182
):
3283
assert client.app
3384

@@ -44,7 +95,7 @@ async def test_testit(
4495
description="This is a test case",
4596
)
4697
)
47-
assert case_id
98+
assert case_id == _IXBUG_DUMMY
4899

49100
status = await fogbugz_client.get_case_status(case_id)
50101
assert status == "Active"

0 commit comments

Comments
 (0)