Skip to content

Commit

Permalink
fix(python): multipart file upload parameter fixes, python core utili…
Browse files Browse the repository at this point in the history 
…ty bugfix. (#5201)

* fix(python): omitted body parameters should not be encoded during files construction.

* fix(python): with_content_type should resolve to specified file content type when provided.

* Update `test_file.py` and improve `with_content_type` definition.

* Add docstring to with_content_type.

* Update python sdk seed snapshots.

* Minor fix; update `file-upload` seed test snapshot to include new changes.

* Snapshot updates.

* Snapshot updates.

* Update snapshots.

* Update ts and java snapshots.

* update
  • Loading branch information
@eyw520
eyw520 authored Nov 19, 2024
1 parent 332a580 commit e056fb4
Show file tree
Hide file tree
Showing 228 changed files with 4,231 additions and 2,178 deletions.
21 changes: 13 additions & 8 deletions generators/python/core_utilities/shared/file.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,25 @@ def convert_file_dict_to_httpx_tuples(
return httpx_tuples


def with_content_type(*, file: File, content_type: str) -> File:
""" """
def with_content_type(*, file: File, default_content_type: str) -> File:
"""
This function resolves to the file's content type, if provided, and defaults
to the default_content_type value if not.
"""
if isinstance(file, tuple):
if len(file) == 2:
filename, content = cast(Tuple[Optional[str], FileContent], file) # type: ignore
return (filename, content, content_type)
return (filename, content, default_content_type)
elif len(file) == 3:
filename, content, _ = cast(Tuple[Optional[str], FileContent, Optional[str]], file) # type: ignore
return (filename, content, content_type)
filename, content, file_content_type = cast(Tuple[Optional[str], FileContent, Optional[str]], file) # type: ignore
out_content_type = file_content_type or default_content_type
return (filename, content, out_content_type)
elif len(file) == 4:
filename, content, _, headers = cast( # type: ignore
filename, content, file_content_type, headers = cast( # type: ignore
Tuple[Optional[str], FileContent, Optional[str], Mapping[str, str]], file
)
return (filename, content, content_type, headers)
out_content_type = file_content_type or default_content_type
return (filename, content, out_content_type, headers)
else:
raise ValueError(f"Unexpected tuple length: {len(file)}")
return (None, file, content_type)
return (None, file, default_content_type)
39 changes: 30 additions & 9 deletions ...ators/sdk/client_generator/request_body_parameters/file_upload_request_body_parameters.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,29 +132,50 @@ def write(writer: AST.NodeWriter) -> None:
writer.write_line("{")
with writer.indent():
for property in self._request.properties:
type_hint = self._get_property_type(property)
property_as_union = property.get_as_union()
if property_as_union.type == "bodyProperty" and property_as_union.content_type is not None:
writer.write(f'"{property_as_union.name.wire_value}": (None, ')
writer.write_node(
AST.Expression(
Json.dumps(
if type_hint.is_optional:
writer.write_line("**(")
with writer.indent():
writer.write(f'{{"{property_as_union.name.wire_value}": (None, ')
writer.write_node(
AST.Expression(
self._context.core_utilities.jsonable_encoder(
AST.Expression(property_as_union.name.wire_value)
Json.dumps(
AST.Expression(
self._context.core_utilities.jsonable_encoder(
AST.Expression(property_as_union.name.wire_value)
)
)
)
)
)
writer.write_line(f', "{property_as_union.content_type}")}}')
writer.write_line(f"if {property_as_union.name.wire_value} is not OMIT ")
writer.write_line("else {}")
writer.write_line("),")
else:
writer.write(f'"{property_as_union.name.wire_value}": (None, ')
writer.write_node(
AST.Expression(
Json.dumps(
AST.Expression(
self._context.core_utilities.jsonable_encoder(
AST.Expression(property_as_union.name.wire_value)
)
)
)
)
)
)
writer.write_line(f', "{property_as_union.content_type}"),')
writer.write_line(f', "{property_as_union.content_type}"),')
elif property_as_union.type == "file":
file_property_as_union = property_as_union.value.get_as_union()
if file_property_as_union.content_type is not None:
writer.write(f'"{file_property_as_union.key.wire_value}": ')
writer.write_node(
self._context.core_utilities.with_content_type(
AST.Expression(
f'file={file_property_as_union.key.wire_value}, content_type="{file_property_as_union.content_type}"'
f'file={file_property_as_union.key.wire_value}, default_content_type="{file_property_as_union.content_type}"'
)
)
)
Expand Down
36 changes: 24 additions & 12 deletions generators/python/tests/utils/test_file.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,52 @@


def test_file_content_bytes() -> None:
result = with_content_type(file=b"file content", content_type="text/plain")
result = with_content_type(file=b"file content", default_content_type="text/plain")
assert result == (None, b"file content", "text/plain")


def test_file_content_str() -> None:
result = with_content_type(file="file content", content_type="text/plain")
result = with_content_type(file="file content", default_content_type="text/plain")
assert result == (None, "file content", "text/plain")


def test_file_content_io() -> None:
file_like = BytesIO(b"file content")
result = with_content_type(file=file_like, content_type="text/plain")
result = with_content_type(file=file_like, default_content_type="text/plain")
filename, content, contentType = cast(Tuple[Optional[str], FileContent, Optional[str]], result)
assert filename is None
assert content == file_like
assert contentType == "text/plain"


def test_tuple_2() -> None:
result = with_content_type(file=("example.txt", b"file content"), content_type="text/plain")
result = with_content_type(file=("example.txt", b"file content"), default_content_type="text/plain")
assert result == ("example.txt", b"file content", "text/plain")


def test_tuple_3() -> None:
result = with_content_type(file=("example.txt", b"file content", "application/octet-stream"), content_type="text/plain")
assert result == ("example.txt", b"file content", "text/plain")
result = with_content_type(
file=("example.txt", b"file content", "application/octet-stream"), default_content_type="text/plain"
)
assert result == ("example.txt", b"file content", "application/octet-stream")


def test_tuple_4() -> None:
result = with_content_type(file=("example.txt", b"file content", "application/octet-stream", {"X-Custom": "value"}), content_type="text/plain")
assert result == ("example.txt", b"file content", "text/plain", {"X-Custom": "value"})
result = with_content_type(
file=("example.txt", b"file content", "application/octet-stream", {"X-Custom": "value"}),
default_content_type="text/plain",
)
assert result == ("example.txt", b"file content", "application/octet-stream", {"X-Custom": "value"})


def test_none_filename() -> None:
result = with_content_type(file=(None, b"file content"), content_type="text/plain")
result = with_content_type(file=(None, b"file content"), default_content_type="text/plain")
assert result == (None, b"file content", "text/plain")


def test_invalid_tuple_length() -> None:
with pytest.raises(ValueError):
with_content_type(
file=("example.txt", b"file content", "text/plain", {}, "extra"), # type: ignore
content_type="application/json"
)
file=("example.txt", b"file content", "text/plain", {}, "extra"), # type: ignore
default_content_type="application/json",
)
32 changes: 32 additions & 0 deletions packages/cli/dynamic-snippets/src/__test__/test-definitions/file-upload.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1503,6 +1503,38 @@
"_type": "named",
"value": "type_service:MyObject"
}
},
{
"type": "bodyProperty",
"name": {
"name": {
"originalName": "foobar",
"camelCase": {
"unsafeName": "foobar",
"safeName": "foobar"
},
"snakeCase": {
"unsafeName": "foobar",
"safeName": "foobar"
},
"screamingSnakeCase": {
"unsafeName": "FOOBAR",
"safeName": "FOOBAR"
},
"pascalCase": {
"unsafeName": "Foobar",
"safeName": "Foobar"
}
},
"wireValue": "foobar"
},
"typeReference": {
"_type": "optional",
"value": {
"_type": "named",
"value": "type_service:MyObject"
}
}
}
]
}
Expand Down
102 changes: 102 additions & 0 deletions packages/cli/generation/ir-generator/src/__test__/test-definitions/file-upload.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2011,6 +2011,108 @@
},
"availability": null,
"docs": null
},
{
"type": "bodyProperty",
"contentType": "application/json",
"name": {
"name": {
"originalName": "foobar",
"camelCase": {
"unsafeName": "foobar",
"safeName": "foobar"
},
"snakeCase": {
"unsafeName": "foobar",
"safeName": "foobar"
},
"screamingSnakeCase": {
"unsafeName": "FOOBAR",
"safeName": "FOOBAR"
},
"pascalCase": {
"unsafeName": "Foobar",
"safeName": "Foobar"
}
},
"wireValue": "foobar"
},
"valueType": {
"_type": "container",
"container": {
"_type": "optional",
"optional": {
"_type": "named",
"name": {
"originalName": "MyObject",
"camelCase": {
"unsafeName": "myObject",
"safeName": "myObject"
},
"snakeCase": {
"unsafeName": "my_object",
"safeName": "my_object"
},
"screamingSnakeCase": {
"unsafeName": "MY_OBJECT",
"safeName": "MY_OBJECT"
},
"pascalCase": {
"unsafeName": "MyObject",
"safeName": "MyObject"
}
},
"fernFilepath": {
"allParts": [
{
"originalName": "service",
"camelCase": {
"unsafeName": "service",
"safeName": "service"
},
"snakeCase": {
"unsafeName": "service",
"safeName": "service"
},
"screamingSnakeCase": {
"unsafeName": "SERVICE",
"safeName": "SERVICE"
},
"pascalCase": {
"unsafeName": "Service",
"safeName": "Service"
}
}
],
"packagePath": [],
"file": {
"originalName": "service",
"camelCase": {
"unsafeName": "service",
"safeName": "service"
},
"snakeCase": {
"unsafeName": "service",
"safeName": "service"
},
"screamingSnakeCase": {
"unsafeName": "SERVICE",
"safeName": "SERVICE"
},
"pascalCase": {
"unsafeName": "Service",
"safeName": "Service"
}
}
},
"typeId": "type_service:MyObject",
"default": null,
"inline": null
}
}
},
"availability": null,
"docs": null
}
],
"docs": null
Expand Down
12 changes: 12 additions & 0 deletions packages/cli/register/src/ir-to-fdr-converter/__test__/__snapshots__/file-upload.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,18 @@
"value": "type_service:MyObject"
},
"contentType": "application/json"
},
{
"type": "bodyProperty",
"key": "foobar",
"valueType": {
"type": "optional",
"itemType": {
"type": "id",
"value": "type_service:MyObject"
}
},
"contentType": "application/json"
}
]
}
Expand Down
3 changes: 3 additions & 0 deletions seed/fastapi/file-upload/.mock/definition/service.yml
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

9 changes: 8 additions & 1 deletion seed/fastapi/file-upload/resources/service/service/service.py
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 5 additions & 6 deletions seed/fastapi/grpc-proto-exhaustive/.mock/generators.yml
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit e056fb4

Please sign in to comment.