Skip to content

Commit 60346c9

Browse files
fix: route RequestBodyPlainText to request_body_data instead of request_body_json (#971)
Co-authored-by: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com>
1 parent 4aaafcf commit 60346c9

File tree

4 files changed

+62
-24
lines changed

4 files changed

+62
-24
lines changed

airbyte_cdk/manifest_migrations/migrations/http_requester_request_body_json_data_to_request_body.py

Lines changed: 41 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,23 @@ class HttpRequesterRequestBodyJsonDataToRequestBody(ManifestMigration):
1515
This migration is responsible for migrating the `request_body_json` and `request_body_data` keys
1616
to a unified `request_body` key in the HttpRequester component.
1717
The migration will copy the value of either original key to `request_body` and remove the original key.
18+
19+
**String-valued `request_body_json` is intentionally left unmigrated.**
20+
21+
When `request_body_json` is a string (e.g. a Jinja template like
22+
`'{"nested": {"key": "{{ config.option }}"}}'`), it is NOT converted to a
23+
`RequestBodyPlainText` or any other typed `request_body` object. This is because:
24+
25+
1. The `InterpolatedRequestOptionsProvider` already handles string `request_body_json`
26+
natively via `InterpolatedNestedRequestInputProvider`, which interpolates the
27+
template and parses the result into a dict using `ast.literal_eval`, then sends
28+
it as a JSON body.
29+
2. Converting it to `RequestBodyPlainText` would route it to `request_body_data`
30+
(raw string body) instead of `request_body_json` (JSON body), breaking connectors
31+
that rely on the body being sent as JSON with the correct Content-Type header.
32+
3. We cannot convert it to `RequestBodyJsonObject` because migrations run before
33+
interpolation, so Jinja templates have not been resolved yet and the string
34+
cannot be parsed into a dict at migration time.
1835
"""
1936

2037
component_type = "HttpRequester"
@@ -26,9 +43,14 @@ class HttpRequesterRequestBodyJsonDataToRequestBody(ManifestMigration):
2643
replacement_key = "request_body"
2744

2845
def should_migrate(self, manifest: ManifestType) -> bool:
29-
return manifest[TYPE_TAG] == self.component_type and any(
30-
key in list(manifest.keys()) for key in self.original_keys
31-
)
46+
if manifest[TYPE_TAG] != self.component_type:
47+
return False
48+
for key in self.original_keys:
49+
if key in manifest:
50+
if key == self.body_json_key and isinstance(manifest[key], str):
51+
continue
52+
return True
53+
return False
3254

3355
def migrate(self, manifest: ManifestType) -> None:
3456
for key in self.original_keys:
@@ -38,21 +60,33 @@ def migrate(self, manifest: ManifestType) -> None:
3860
self._migrate_body_data(manifest, key)
3961

4062
def validate(self, manifest: ManifestType) -> bool:
41-
return self.replacement_key in manifest and all(
42-
key not in manifest for key in self.original_keys
63+
has_replacement = self.replacement_key in manifest
64+
has_unmigrated = any(
65+
key in manifest
66+
for key in self.original_keys
67+
if not (key == self.body_json_key and isinstance(manifest.get(key), str))
68+
)
69+
has_string_json = self.body_json_key in manifest and isinstance(
70+
manifest[self.body_json_key], str
4371
)
72+
if has_string_json:
73+
return not has_unmigrated
74+
return has_replacement and not has_unmigrated
4475

4576
def _migrate_body_json(self, manifest: ManifestType, key: str) -> None:
4677
"""
4778
Migrate the value of the request_body_json.
79+
80+
String values are left as-is (not migrated) because they are Jinja templates
81+
that will be interpolated and parsed into dicts at runtime by
82+
InterpolatedNestedRequestInputProvider. See class docstring for details.
4883
"""
4984
query_key = "query"
50-
text_type = "RequestBodyPlainText"
5185
graph_ql_type = "RequestBodyGraphQL"
5286
json_object_type = "RequestBodyJsonObject"
5387

5488
if isinstance(manifest[key], str):
55-
self._migrate_value(manifest, key, text_type)
89+
return
5690
elif isinstance(manifest[key], dict):
5791
if isinstance(manifest[key].get(query_key), str):
5892
self._migrate_value(manifest, key, graph_ql_type)

airbyte_cdk/sources/declarative/requesters/request_options/interpolated_request_options_provider.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,9 @@ def _resolve_request_body(self) -> None:
101101
self.request_body_data = self.request_body.value
102102
elif self.request_body.type == "RequestBodyGraphQL":
103103
self.request_body_json = self.request_body.value.dict(exclude_none=True)
104-
elif self.request_body.type in ("RequestBodyJsonObject", "RequestBodyPlainText"):
104+
elif self.request_body.type == "RequestBodyPlainText":
105+
self.request_body_data = self.request_body.value
106+
elif self.request_body.type == "RequestBodyJsonObject":
105107
self.request_body_json = self.request_body.value
106108
else:
107109
raise ValueError(f"Unsupported request body type: {self.request_body.type}")

unit_tests/manifest_migrations/conftest.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -941,10 +941,7 @@ def expected_manifest_with_migrated_to_request_body() -> Dict[str, Any]:
941941
"type": "HttpRequester",
942942
"http_method": "GET",
943943
"url": "https://example.com/v2/path_to_B",
944-
"request_body": {
945-
"type": "RequestBodyPlainText",
946-
"value": '{"nested": { "key": "{{ config[\'option\'] }}" }}',
947-
},
944+
"request_body_json": '{"nested": { "key": "{{ config[\'option\'] }}" }}',
948945
},
949946
"record_selector": {
950947
"type": "RecordSelector",
@@ -1102,10 +1099,7 @@ def expected_manifest_with_migrated_to_request_body() -> Dict[str, Any]:
11021099
"type": "HttpRequester",
11031100
"http_method": "GET",
11041101
"url": "https://example.com/v2/path_to_B",
1105-
"request_body": {
1106-
"type": "RequestBodyPlainText",
1107-
"value": '{"nested": { "key": "{{ config[\'option\'] }}" }}',
1108-
},
1102+
"request_body_json": '{"nested": { "key": "{{ config[\'option\'] }}" }}',
11091103
},
11101104
"record_selector": {
11111105
"type": "RecordSelector",

unit_tests/sources/declarative/requesters/request_options/test_interpolated_request_options_provider.py

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -211,14 +211,6 @@ def test_interpolated_request_json(test_name, input_request_json, expected_reque
211211
RequestBodyJsonObject(type="RequestBodyJsonObject", value={"none_value": "{{ None }}"}),
212212
{},
213213
),
214-
(
215-
"test_string",
216-
RequestBodyPlainText(
217-
type="RequestBodyPlainText",
218-
value="""{"nested": { "key": "{{ config['option'] }}" }}""",
219-
),
220-
{"nested": {"key": "OPTION"}},
221-
),
222214
(
223215
"test_nested_objects",
224216
RequestBodyJsonObject(
@@ -345,6 +337,22 @@ def test_interpolated_request_data(test_name, input_request_data, expected_reque
345337
),
346338
{"2020-01-01 - 12345": "ABC"},
347339
),
340+
(
341+
"test_plain_text_body",
342+
RequestBodyPlainText(
343+
type="RequestBodyPlainText",
344+
value="plain text body content",
345+
),
346+
"plain text body content",
347+
),
348+
(
349+
"test_plain_text_with_interpolation",
350+
RequestBodyPlainText(
351+
type="RequestBodyPlainText",
352+
value="interpolate_me=5&option={{ config['option'] }}",
353+
),
354+
"interpolate_me=5&option=OPTION",
355+
),
348356
],
349357
)
350358
def test_interpolated_request_data_using_request_body(

0 commit comments

Comments
 (0)