Skip to content

Commit 34e1a67

Browse files
xirzecjeremymeng
xirzec
and
jeremymeng
authored
[core-client] Fix serializer issue with nested polymorphics. (Azure#19455)
* Fix serializer issue with nested polymorphics. The Media Services SDK was having some serialization related issues because of some edge cases in how we handle polymorphic types. Specifically, when a Sequence property of a polymorphic type has an element that is also a polymorphic type. The issue stems from the fact that the `element` declaration inside Composite properties is a subset of the full definition, that is to say it *only* has the properties `name` and `className`, whereas the full Mapper would also declare things like `uberParent` and polymorphicDiscriminator`. Because of this, we wouldn't correctly look up the polymorphic type of the elements in the array and instead serialize them as their parent type. The subtlety is we *would* try to look up the discriminator, but without the `uberParent` definition, we would fall back to the current className, which would produce the wrong identifier for use in the `discriminators` map Separately, I noticed we weren't properly deserializing items because of how `serializedName` gets escaped by the generator. Furthermore, we again weren't correctly looking up the type, but this wasn't easily noticeable because the deserializer by default will preserve unknown properties, but those properties weren't getting correctly deserialized either. * Fix linting errors * format * Rename testMappers to testMappers1 Co-authored-by: Jeremy Meng <yumeng@microsoft.com>
1 parent ab56608 commit 34e1a67

File tree

6 files changed

+638
-6
lines changed

6 files changed

+638
-6
lines changed

‎sdk/core/core-client/.vscode/launch.json‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
{
2+
// Use IntelliSense to learn about possible attributes.
3+
// Hover to view descriptions of existing attributes.
4+
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
5+
"version": "0.2.0",
6+
"configurations": [
7+
{
8+
"type": "node",
9+
"request": "launch",
10+
"name": "Debug Mocha Test [Without Rollup]",
11+
"program": "${workspaceFolder}/node_modules/mocha/bin/_mocha",
12+
"args": [
13+
"-r",
14+
"esm",
15+
"-r",
16+
"ts-node/register",
17+
"--timeout",
18+
"999999",
19+
"--colors",
20+
"--exclude",
21+
"${workspaceFolder}/test/**/browser/*.spec.ts",
22+
"${workspaceFolder}/test/**/*.spec.ts"
23+
],
24+
"env": {
25+
// "TS_NODE_COMPILER_OPTIONS": "{\"module\": \"commonjs\"}",
26+
"TS_NODE_FILES": "true",
27+
"NODE_PATH": "${workspaceFolder}/../../../common/temp/node_modules/.pnpm/node_modules"
28+
},
29+
"skipFiles": ["<node_internals>/**"],
30+
"console": "integratedTerminal",
31+
"internalConsoleOptions": "neverOpen",
32+
"protocol": "inspector"
33+
}
34+
]
35+
}

‎sdk/core/core-client/src/serializer.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -555,13 +555,19 @@ function serializeSequenceType(
555555
if (!Array.isArray(object)) {
556556
throw new Error(`${objectName} must be of type Array.`);
557557
}
558-
const elementType = mapper.type.element;
558+
let elementType = mapper.type.element;
559559
if (!elementType || typeof elementType !== "object") {
560560
throw new Error(
561561
`element" metadata for an Array must be defined in the ` +
562562
`mapper and it must of type "object" in ${objectName}.`
563563
);
564564
}
565+
// Quirk: Composite mappers referenced by `element` might
566+
// not have *all* properties declared (like uberParent),
567+
// so let's try to look up the full definition by name.
568+
if (elementType.type.name === "Composite" && elementType.type.className) {
569+
elementType = serializer.modelMappers[elementType.type.className] ?? elementType;
570+
}
565571
const tempArray = [];
566572
for (let i = 0; i < object.length; i++) {
567573
const serializedValue = serializer.serialize(elementType, object[i], objectName, options);
@@ -1056,8 +1062,7 @@ function deserializeSequenceType(
10561062
objectName: string,
10571063
options: RequiredSerializerOptions
10581064
): any {
1059-
/* jshint validthis: true */
1060-
const element = mapper.type.element;
1065+
let element = mapper.type.element;
10611066
if (!element || typeof element !== "object") {
10621067
throw new Error(
10631068
`element" metadata for an Array must be defined in the ` +
@@ -1070,6 +1075,13 @@ function deserializeSequenceType(
10701075
responseBody = [responseBody];
10711076
}
10721077

1078+
// Quirk: Composite mappers referenced by `element` might
1079+
// not have *all* properties declared (like uberParent),
1080+
// so let's try to look up the full definition by name.
1081+
if (element.type.name === "Composite" && element.type.className) {
1082+
element = serializer.modelMappers[element.type.className] ?? element;
1083+
}
1084+
10731085
const tempArray = [];
10741086
for (let i = 0; i < responseBody.length; i++) {
10751087
tempArray[i] = serializer.deserialize(
@@ -1092,8 +1104,12 @@ function getPolymorphicMapper(
10921104
): CompositeMapper {
10931105
const polymorphicDiscriminator = getPolymorphicDiscriminatorRecursively(serializer, mapper);
10941106
if (polymorphicDiscriminator) {
1095-
const discriminatorName = polymorphicDiscriminator[polymorphicPropertyName];
1107+
let discriminatorName = polymorphicDiscriminator[polymorphicPropertyName];
10961108
if (discriminatorName) {
1109+
// The serializedName might have \\, which we just want to ignore
1110+
if (polymorphicPropertyName === "serializedName") {
1111+
discriminatorName = discriminatorName.replace(/\\/gi, "");
1112+
}
10971113
const discriminatorValue = object[discriminatorName];
10981114
if (discriminatorValue !== undefined && discriminatorValue !== null) {
10991115
const typeName = mapper.type.uberParent || mapper.type.className;

‎sdk/core/core-client/test/serializationPolicy.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { createPipelineRequest } from "@azure/core-rest-pipeline";
66
import { stringifyXML } from "@azure/core-xml";
77
import { createSerializer, MapperTypeNames } from "../src";
88
import { serializeRequestBody, serializeHeaders } from "../src/serializationPolicy";
9-
import { Mappers } from "./testMappers";
9+
import { Mappers } from "./testMappers1";
1010

1111
describe("serializationPolicy", function () {
1212
describe("serializeRequestBody()", () => {

‎sdk/core/core-client/test/serializer.spec.ts‎

Lines changed: 152 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,8 @@ import {
1010
DictionaryMapper,
1111
CompositeMapper,
1212
} from "../src";
13-
import { Mappers } from "./testMappers";
13+
import { Mappers } from "./testMappers1";
14+
import * as MediaMappers from "./testMappers2";
1415

1516
const Serializer = createSerializer(Mappers);
1617
const valid_uuid = "ceaafd1e-f936-429f-bbfc-82ee75dddc33";
@@ -828,6 +829,81 @@ describe("Serializer", function () {
828829
});
829830
});
830831

832+
it("should correctly serialize polymorphic children of a sequence of polymorphic elements", function () {
833+
const bumperJobInputAsset = {
834+
odataType: "#Microsoft.Media.JobInputAsset",
835+
assetName: "input2",
836+
start: {
837+
odataType: "#Microsoft.Media.AbsoluteClipTime",
838+
time: "PT0S",
839+
},
840+
label: "bumper",
841+
};
842+
843+
const mainJobInputAsset = {
844+
odataType: "#Microsoft.Media.JobInputAsset",
845+
assetName: "input",
846+
start: {
847+
odataType: "#Microsoft.Media.AbsoluteClipTime",
848+
time: "PT0S",
849+
},
850+
label: "main",
851+
};
852+
853+
const input = {
854+
odataType: "#Microsoft.Media.JobInputSequence",
855+
inputs: [bumperJobInputAsset, mainJobInputAsset],
856+
};
857+
const outputs = [
858+
{
859+
odataType: "#Microsoft.Media.JobOutputAsset",
860+
assetName: "outputAssetName",
861+
},
862+
];
863+
const requestBody = {
864+
input,
865+
outputs,
866+
};
867+
868+
const MediaSerializer = createSerializer(MediaMappers);
869+
const result = MediaSerializer.serialize(MediaMappers.Job, requestBody);
870+
// assetName can get clipped off if this fails, since input.inputs
871+
// elements will get serialized as JobInputClip instead of JobInputAsset
872+
assert.deepStrictEqual(result, {
873+
properties: {
874+
input: {
875+
"@odata.type": "#Microsoft.Media.JobInputSequence",
876+
inputs: [
877+
{
878+
"@odata.type": "#Microsoft.Media.JobInputAsset",
879+
start: {
880+
"@odata.type": "#Microsoft.Media.AbsoluteClipTime",
881+
time: "PT0S",
882+
},
883+
label: "bumper",
884+
assetName: "input2",
885+
},
886+
{
887+
"@odata.type": "#Microsoft.Media.JobInputAsset",
888+
start: {
889+
"@odata.type": "#Microsoft.Media.AbsoluteClipTime",
890+
time: "PT0S",
891+
},
892+
label: "main",
893+
assetName: "input",
894+
},
895+
],
896+
},
897+
outputs: [
898+
{
899+
"@odata.type": "#Microsoft.Media.JobOutputAsset",
900+
assetName: "outputAssetName",
901+
},
902+
],
903+
},
904+
});
905+
});
906+
831907
describe("deserialize", function () {
832908
it("should correctly deserialize a Date if the type is 'any'", function () {
833909
const mapper: Mapper = {
@@ -1790,5 +1866,80 @@ describe("Serializer", function () {
17901866
assert.equal(result.siblings[2].jawsize, 5);
17911867
});
17921868
});
1869+
1870+
it("should correctly deserialize polymorphic children of a sequence of polymorphic elements", function () {
1871+
const serializedPayload = {
1872+
properties: {
1873+
input: {
1874+
"@odata.type": "#Microsoft.Media.JobInputSequence",
1875+
inputs: [
1876+
{
1877+
"@odata.type": "#Microsoft.Media.JobInputAsset",
1878+
start: {
1879+
"@odata.type": "#Microsoft.Media.AbsoluteClipTime",
1880+
time: "PT0S",
1881+
},
1882+
label: "bumper",
1883+
assetName: "input2",
1884+
},
1885+
{
1886+
"@odata.type": "#Microsoft.Media.JobInputAsset",
1887+
start: {
1888+
"@odata.type": "#Microsoft.Media.AbsoluteClipTime",
1889+
time: "PT0S",
1890+
},
1891+
label: "main",
1892+
assetName: "input",
1893+
},
1894+
],
1895+
},
1896+
outputs: [
1897+
{
1898+
"@odata.type": "#Microsoft.Media.JobOutputAsset",
1899+
assetName: "outputAssetName",
1900+
},
1901+
],
1902+
},
1903+
};
1904+
1905+
const MediaSerializer = createSerializer(MediaMappers);
1906+
const result = MediaSerializer.deserialize(
1907+
MediaMappers.Job,
1908+
serializedPayload,
1909+
"anyResponseBody"
1910+
);
1911+
// This can fail by "@odata.type" properties not turning into odataType
1912+
assert.deepStrictEqual(result, {
1913+
input: {
1914+
odataType: "#Microsoft.Media.JobInputSequence",
1915+
inputs: [
1916+
{
1917+
odataType: "#Microsoft.Media.JobInputAsset",
1918+
start: {
1919+
odataType: "#Microsoft.Media.AbsoluteClipTime",
1920+
time: "PT0S",
1921+
},
1922+
label: "bumper",
1923+
assetName: "input2",
1924+
},
1925+
{
1926+
odataType: "#Microsoft.Media.JobInputAsset",
1927+
start: {
1928+
odataType: "#Microsoft.Media.AbsoluteClipTime",
1929+
time: "PT0S",
1930+
},
1931+
label: "main",
1932+
assetName: "input",
1933+
},
1934+
],
1935+
},
1936+
outputs: [
1937+
{
1938+
odataType: "#Microsoft.Media.JobOutputAsset",
1939+
assetName: "outputAssetName",
1940+
},
1941+
],
1942+
});
1943+
});
17931944
});
17941945
});

sdk/core/core-client/test/testMappers.ts renamed to sdk/core/core-client/test/testMappers1.ts

File renamed without changes.

0 commit comments

Comments
 (0)