Skip to content

Commit

Permalink
[http-client-python] enhance the xml impl (#5701)
Browse files Browse the repository at this point in the history
1. use new serialization options from tcgc
2. add error deserialization support

fix: Azure/autorest.python#2921
fix: Azure/autorest.python#2922
  • Loading branch information
tadelesh authored Jan 23, 2025
1 parent 3210f18 commit 8567833
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 72 deletions.
5 changes: 1 addition & 4 deletions packages/http-client-python/emitter/src/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,7 @@ async function createPythonSdkContext<TServiceOperation extends SdkServiceOperat
return {
...(await createSdkContext<PythonEmitterOptions, TServiceOperation>(
context,
"@typespec/http-client-python",
{
additionalDecorators: ["TypeSpec\\.@encodedName"],
},
"@azure-tools/typespec-python",
)),
__endpointPathParameters: [],
};
Expand Down
82 changes: 25 additions & 57 deletions packages/http-client-python/emitter/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
SdkEndpointType,
SdkEnumType,
SdkEnumValueType,
SdkModelPropertyType,
SdkModelType,
SdkServiceOperation,
SdkType,
Expand Down Expand Up @@ -242,7 +241,7 @@ function emitProperty<TServiceOperation extends SdkServiceOperation>(
}
return {
clientName: camelToSnakeCase(property.name),
wireName: property.serializedName,
wireName: property.serializationOptions.json?.name ?? property.name,
type: getType(context, sourceType),
optional: property.optional,
description: property.summary ? property.summary : property.doc,
Expand All @@ -251,7 +250,7 @@ function emitProperty<TServiceOperation extends SdkServiceOperation>(
isDiscriminator: property.discriminator,
flatten: property.flatten,
isMultipartFileInput: isMultipartFileInput,
xmlMetadata: model.usage & UsageFlags.Xml ? getXmlMetadata(property) : undefined,
xmlMetadata: getXmlMetadata(property),
};
}

Expand Down Expand Up @@ -294,7 +293,7 @@ function emitModel<TServiceOperation extends SdkServiceOperation>(
crossLanguageDefinitionId: type.crossLanguageDefinitionId,
usage: type.usage,
isXml: type.usage & UsageFlags.Xml ? true : false,
xmlMetadata: type.usage & UsageFlags.Xml ? getXmlMetadata(type) : undefined,
xmlMetadata: getXmlMetadata(type),
clientNamespace: getClientNamespace(context, type.clientNamespace),
};

Expand Down Expand Up @@ -528,58 +527,27 @@ export function emitEndpointType<TServiceOperation extends SdkServiceOperation>(
return params;
}

function getXmlMetadata(type: SdkType | SdkModelPropertyType): Record<string, any> {
const xmlMetadata: Record<string, any> = {};
const xmlDecorators = type.decorators.filter(
(x) => x.name.startsWith("TypeSpec.Xml.") || x.name.startsWith("TypeSpec.@encodedName"),
);
for (const decorator of xmlDecorators) {
switch (decorator.name) {
case "TypeSpec.@encodedName":
if (decorator.arguments["mimeType"] === "application/xml") {
xmlMetadata["name"] = decorator.arguments["name"];
break;
}
continue;
case "TypeSpec.Xml.@attribute":
xmlMetadata["attribute"] = true;
break;
case "TypeSpec.Xml.@name":
xmlMetadata["name"] = decorator.arguments["name"];
break;
case "TypeSpec.Xml.@ns":
if (decorator.arguments["ns"].kind === "enumvalue") {
xmlMetadata["namespace"] = (decorator.arguments["ns"] as SdkEnumValueType).value;
xmlMetadata["prefix"] = (decorator.arguments["ns"] as SdkEnumValueType).name;
} else {
xmlMetadata["namespace"] = decorator.arguments["ns"];
xmlMetadata["prefix"] = decorator.arguments["prefix"];
}
break;
case "TypeSpec.Xml.@unwrapped":
if (type.kind === "property" && type.type.kind === "array") {
xmlMetadata["unwrapped"] = true;
} else {
xmlMetadata["text"] = true;
}
break;
}
}
// add item metadata for array
if (
type.kind === "property" &&
type.type.kind === "array" &&
type.type.valueType.kind !== "model"
) {
const itemMetadata = getXmlMetadata(type.type.valueType);
// if array item is a primitive type, we need to use itemsName to change the name
if (Object.keys(itemMetadata).length > 0) {
xmlMetadata["itemsName"] = itemMetadata["name"];
xmlMetadata["itemsNs"] = itemMetadata["namespace"];
xmlMetadata["itemsPrefix"] = itemMetadata["prefix"];
} else if (!xmlMetadata["unwrapped"]) {
xmlMetadata["itemsName"] = type.type.valueType.kind;
}
function getXmlMetadata(
type: SdkModelType | SdkBodyModelPropertyType,
): Record<string, any> | undefined {
if (type.serializationOptions.xml) {
return {
name: type.serializationOptions.xml.name,
namespace: type.serializationOptions.xml.ns?.namespace,
prefix: type.serializationOptions.xml.ns?.prefix,
attribute: type.serializationOptions.xml.attribute,
unwrapped:
type.kind === "property" &&
type.type.kind === "array" &&
type.serializationOptions.xml.unwrapped,
text:
type.kind === "property" &&
type.type.kind !== "array" &&
type.serializationOptions.xml.unwrapped,
itemsName: type.serializationOptions.xml.itemsName,
itemsNs: type.serializationOptions.xml.itemsNs?.namespace,
itemsPrefix: type.serializationOptions.xml.itemsNs?.prefix,
};
}
return xmlMetadata;
return undefined;
}
Original file line number Diff line number Diff line change
Expand Up @@ -456,7 +456,7 @@ def imports( # pylint: disable=too-many-branches, disable=too-many-statements
ImportType.LOCAL,
)
file_import.add_import("json", ImportType.STDLIB)
if any(xml_serializable(str(r.default_content_type)) for r in self.responses):
if any(xml_serializable(str(r.default_content_type)) for r in self.responses + self.exceptions):
file_import.add_submodule_import(relative_path, "_deserialize_xml", ImportType.LOCAL)
elif self.need_deserialize:
file_import.add_submodule_import(relative_path, "_deserialize", ImportType.LOCAL)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1005,7 +1005,7 @@ def response_deserialization( # pylint: disable=too-many-statements
retval.extend(deserialize_code)
return retval

def handle_error_response(self, builder: OperationType) -> List[str]:
def handle_error_response(self, builder: OperationType) -> List[str]: # pylint: disable=too-many-statements, too-many-branches
async_await = "await " if self.async_mode else ""
retval = [f"if response.status_code not in {str(builder.success_status_codes)}:"]
response_read = [
Expand Down Expand Up @@ -1079,7 +1079,14 @@ def handle_error_response(self, builder: OperationType) -> List[str]:
is_operation_file=True, skip_quote=True, serialize_namespace=self.serialize_namespace
)
if self.code_model.options["models_mode"] == "dpg":
retval.append(f" error = _failsafe_deserialize({type_annotation}, response.json())")
if xml_serializable(str(e.default_content_type)):
retval.append(
f" error = _failsafe_deserialize_xml({type_annotation}, response.text())"
)
else:
retval.append(
f" error = _failsafe_deserialize({type_annotation}, response.json())"
)
else:
retval.append(
f" error = self._deserialize.failsafe_deserialize({type_annotation}, "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -956,6 +956,20 @@ def _failsafe_deserialize(
return None


def _failsafe_deserialize_xml(
deserializer: typing.Any,
value: typing.Any,
) -> typing.Any:
try:
return _deserialize_xml(deserializer, value)
except DeserializationError:
_LOGGER.warning(
"Ran into a deserialization error. Ignoring since this is failsafe deserialization",
exc_info=True
)
return None


class _RestField:
def __init__(
self,
Expand Down
13 changes: 7 additions & 6 deletions packages/http-client-python/package-lock.json

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

4 changes: 2 additions & 2 deletions packages/http-client-python/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"@azure-tools/typespec-azure-resource-manager": ">=0.50.0 <1.0.0",
"@azure-tools/typespec-autorest": ">=0.50.0 <1.0.0",
"@azure-tools/typespec-azure-rulesets": ">=0.50.0 <3.0.0",
"@azure-tools/typespec-client-generator-core": ">=0.50.0 <1.0.0"
"@azure-tools/typespec-client-generator-core": ">=0.50.2 <1.0.0"
},
"dependencies": {
"js-yaml": "~4.1.0",
Expand All @@ -78,7 +78,7 @@
"@azure-tools/typespec-azure-core": "~0.50.0",
"@azure-tools/typespec-azure-rulesets": "~0.50.0",
"@azure-tools/typespec-azure-resource-manager": "~0.50.0",
"@azure-tools/typespec-client-generator-core": "~0.50.0",
"@azure-tools/typespec-client-generator-core": "~0.50.2",
"@azure-tools/azure-http-specs": "0.1.0-alpha.5",
"@typespec/http-specs": "0.1.0-alpha.7",
"@types/js-yaml": "~4.0.5",
Expand Down

0 comments on commit 8567833

Please sign in to comment.