Skip to content

Commit 5869392

Browse files
committed
remove dedupe lookup logic
Signed-off-by: Alina Buzachis <[email protected]>
1 parent 75d280c commit 5869392

File tree

2 files changed

+16
-161
lines changed

2 files changed

+16
-161
lines changed

core/tests/test_controller_client.py

Lines changed: 13 additions & 108 deletions
Original file line numberDiff line numberDiff line change
@@ -28,128 +28,33 @@ def _fake_response(status_code: int, payload: dict | list) -> requests.Response:
2828
return resp
2929

3030

31-
@patch("core.utils.controller.client.get_http_session")
32-
def test_post_success(mock_get_http_session):
33-
session = MagicMock()
34-
session.post.return_value = _fake_response(201, {"id": 99})
35-
mock_get_http_session.return_value = session
36-
37-
assert cc.post(session, "/labels/", {"name": "foo"}) == {"id": 99}
38-
session.post.assert_called_once()
39-
40-
41-
@patch("core.utils.controller.client.get_http_session")
42-
def test_post_duplicate_name(mock_get_http_session):
43-
session = MagicMock()
44-
session.post.return_value = _fake_response(400, {"error": "duplicate"})
45-
session.get.return_value = _fake_response(
46-
200, {"results": [{"id": 42, "name": "foo"}]}
47-
)
48-
mock_get_http_session.return_value = session
49-
50-
out = cc.post(session, "/labels/", {"name": "foo"})
51-
assert out == {"id": 42, "name": "foo"}
52-
53-
session.get.assert_called_once()
54-
session.post.assert_called_once()
55-
56-
5731
@patch("core.utils.controller.client.get_http_session")
5832
def test_post_non_400_error_is_propagated(mock_get_http_session):
33+
"""
34+
Test that a non-400 error is also immediately propagated.
35+
"""
5936
session = MagicMock()
6037
session.post.return_value = _fake_response(500, {"error": "server"})
6138
mock_get_http_session.return_value = session
6239

6340
with pytest.raises(requests.HTTPError):
6441
cc.post(session, "/labels/", {"name": "foo"})
6542

66-
# Ensure we don't try to deduplicate
6743
session.get.assert_not_called()
44+
session.post.assert_called_once()
6845

6946

7047
@patch("core.utils.controller.client.get_http_session")
71-
def test_post_400_dedupe_no_results_raises(mock_get_http_session):
72-
session = MagicMock()
73-
post_resp = _fake_response(400, {"detail": "duplicate"})
74-
session.post.return_value = post_resp
75-
session.get.return_value = _fake_response(200, {"results": []})
76-
mock_get_http_session.return_value = session
77-
78-
with pytest.raises(requests.HTTPError) as exc:
79-
cc.post(session, "/labels/", {"name": "foo", "organization": 1})
80-
81-
assert isinstance(exc.value, requests.HTTPError)
82-
assert exc.value.response.status_code == 400
83-
session.get.assert_called_once()
84-
# Ensure params include both default dedupe keys
85-
_, kwargs = session.get.call_args
86-
assert kwargs["params"] == {"name": "foo", "organization": 1}
87-
88-
89-
@patch("core.utils.controller.client.get_http_session")
90-
def test_post_400_dedupe_lookup_http_error_still_raises(mock_get_http_session):
91-
session = MagicMock()
92-
session.post.return_value = _fake_response(400, {"detail": "duplicate"})
93-
# Lookup GET itself errors (e.g., 404)
94-
session.get.return_value = _fake_response(404, {"detail": "not found"})
95-
mock_get_http_session.return_value = session
96-
97-
with pytest.raises(requests.HTTPError):
98-
cc.post(session, "/labels/", {"name": "foo"})
99-
100-
session.get.assert_called_once()
101-
102-
103-
@patch("core.utils.controller.client.get_http_session")
104-
def test_post_400_dedupe_params_subset_when_missing_org(mock_get_http_session):
105-
session = MagicMock()
106-
session.post.return_value = _fake_response(400, {"detail": "duplicate"})
107-
session.get.return_value = _fake_response(200, {"results": []})
108-
mock_get_http_session.return_value = session
109-
110-
with pytest.raises(requests.HTTPError):
111-
cc.post(session, "/labels/", {"name": "foo"})
112-
113-
# Only 'name' present in params, no 'organization' since it wasn’t provided
114-
_, kwargs = session.get.call_args
115-
assert kwargs["params"] == {"name": "foo"}
116-
117-
118-
@patch("core.utils.controller.client.get_http_session")
119-
def test_post_400_with_custom_dedupe_keys(mock_get_http_session):
120-
session = MagicMock()
121-
session.post.return_value = _fake_response(400, {"detail": "duplicate"})
122-
session.get.return_value = _fake_response(200, {"results": []})
123-
mock_get_http_session.return_value = session
124-
125-
with pytest.raises(requests.HTTPError):
126-
cc.post(
127-
session,
128-
"/labels/",
129-
{"name": "foo", "organization": 1, "extra": "x"},
130-
dedupe_keys=("name", "extra"),
131-
)
132-
133-
# Only custom keys should be used
134-
_, kwargs = session.get.call_args
135-
assert kwargs["params"] == {"name": "foo", "extra": "x"}
136-
137-
138-
@patch("core.utils.controller.client.get_http_session")
139-
def test_post_400_error_json_fallback_to_text(mock_get_http_session):
48+
def test_post_success(mock_get_http_session):
49+
"""
50+
Test that a successful POST request returns the JSON response.
51+
"""
14052
session = MagicMock()
141-
# Create a response whose .json() raises, forcing fallback to .text
142-
resp = MagicMock(spec=requests.Response)
143-
resp.status_code = 400
144-
resp.text = "not json"
145-
resp.json.side_effect = Exception("bad json")
146-
resp.raise_for_status.side_effect = requests.HTTPError(response=resp)
147-
session.post.return_value = resp
148-
session.get.return_value = _fake_response(200, {"results": []})
53+
session.post.return_value = _fake_response(201, {"id": 123, "name": "foo"})
14954
mock_get_http_session.return_value = session
15055

151-
with pytest.raises(requests.HTTPError) as exc:
152-
cc.post(session, "/labels/", {"name": "foo"})
56+
out = cc.post(session, "/labels/", {"name": "foo"})
57+
assert out == {"id": 123, "name": "foo"}
15358

154-
# Error message should include the fallback text
155-
assert "not json" in str(exc.value)
59+
session.get.assert_not_called()
60+
session.post.assert_called_once()

core/utils/controller/client.py

Lines changed: 3 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,8 @@
1-
import json
21
import logging
32
import urllib.parse
43
from typing import Any
54
from typing import Dict
65
from typing import Optional
7-
from typing import Sequence
86

97
import requests
108
from django.conf import settings
@@ -32,30 +30,17 @@ def get(url: str, *, params: Optional[Dict] = None) -> requests.Response:
3230
return response
3331

3432

35-
def post(
36-
session: requests.Session,
37-
path: str,
38-
data: Dict,
39-
*,
40-
dedupe_keys: Sequence[str] = ("name", "organization"),
41-
) -> Dict[str, Any]:
33+
def post(session: requests.Session, path: str, data: Dict) -> Dict[str, Any]:
4234
"""
4335
Create a resource on the AAP controller.
44-
If the POST fails with 400 because the object already exists,
45-
perform a GET lookup using *dedupe_keys* taken from *data* and
46-
return the existing object instead.
4736
Args:
4837
session: Pre-existing session.
4938
path: Controller endpoint, e.g. "/projects/" (must include trailing slash).
5039
data: JSON payload to send.
51-
dedupe_keys: Keys from *data* to copy into the lookup query string when handling
52-
a duplicate‑name error.
5340
Returns:
5441
JSON for the created or pre‑existing object.
5542
Raises:
5643
requests.HTTPError
57-
Propagated if the status code is not a handled 400 or if the lookup
58-
returns no result.
5944
"""
6045
url = urllib.parse.urljoin(settings.AAP_URL, path)
6146

@@ -64,40 +49,5 @@ def post(
6449
response.raise_for_status()
6550
return safe_json(lambda: response)()
6651

67-
except requests.exceptions.HTTPError as exc:
68-
response = exc.response
69-
if response.status_code != 400:
70-
raise
71-
72-
try:
73-
error_json = safe_json(lambda: response)()
74-
except Exception:
75-
error_json = {"detail": response.text}
76-
77-
logger.warning(f"AAP POST {url} failed with 400. Error response: {error_json}")
78-
logger.debug(f"Payload sent: {json.dumps(data, indent=2)}")
79-
logger.debug(f"AAP POST {url} 400; dedup lookup keys: {dedupe_keys}")
80-
81-
params = {k: data[k] for k in dedupe_keys if k in data}
82-
try:
83-
lookup_resp = session.get(url, params=params)
84-
lookup_resp.raise_for_status()
85-
results = safe_json(lambda: lookup_resp)().get("results", [])
86-
87-
if results:
88-
logger.debug(
89-
f"""Resource already exists. Returning
90-
existing resource: {results[0]}"""
91-
)
92-
return results[0] # type: ignore[no-any-return]
93-
except Exception as e:
94-
logger.debug(
95-
f"Deduplication GET failed for {url} with params {params}: {e}"
96-
)
97-
98-
raise requests.HTTPError(
99-
f"400 Bad Request for {url}.\n"
100-
f"Payload: {json.dumps(data, indent=2)}\n"
101-
f"Response: {json.dumps(error_json, indent=2)}",
102-
response=response,
103-
)
52+
except requests.exceptions.HTTPError:
53+
raise

0 commit comments

Comments
 (0)