[core] Copy existing properties when flattening array in deserializat…

…ion (Azure#15655) Porting fix from Azure/ms-rest-js#451. When flattening array, we were assigning directly to the result instance, thus losing any properties that were deserialized previously in the loop. The fix is to copy the existing properties over. Fixes Azure#15653.
sadasant · Jun 21, 2021 · e452ec1 · e452ec1
1 parent 1d400d5
commit e452ec1
Show file tree

Hide file tree

Showing 8 changed files with 169 additions and 2 deletions.
diff --git a/sdk/core/core-client/CHANGELOG.md b/sdk/core/core-client/CHANGELOG.md
@@ -2,9 +2,14 @@
 
 ## 1.1.3 (Unreleased)
 
+### Key Bugs Fixed
+
+- Fix an issue of lost properties when flattening array in deserialization [issue 15653](https://github.com/azure/azure-sdk-for-js/issues/15653)
 
 ## 1.1.2 (2021-05-20)
 
+### Fixed
+
 - Fixed an issue to check for the mandatory parameter in the header and query values. [PR 15278](https://github.com/Azure/azure-sdk-for-js/pull/15278)
 
 ## 1.1.1 (2021-05-06)

diff --git a/sdk/core/core-client/src/serializer.ts b/sdk/core/core-client/src/serializer.ts
@@ -962,12 +962,20 @@ function deserializeCompositeType(
       // paging
       if (Array.isArray(responseBody[key]) && modelProps[key].serializedName === "") {
         propertyInstance = responseBody[key];
-        instance = serializer.deserialize(
+        const arrayInstance = serializer.deserialize(
           propertyMapper,
           propertyInstance,
           propertyObjectName,
           options
         );
+        // Copy over any properties that have already been added into the instance, where they do
+        // not exist on the newly de-serialized array
+        for (const [k, v] of Object.entries(instance)) {
+          if (!Object.prototype.hasOwnProperty.call(arrayInstance, k)) {
+            arrayInstance[k] = v;
+          }
+        }
+        instance = arrayInstance;
       } else if (propertyInstance !== undefined || propertyMapper.defaultValue !== undefined) {
         serializedValue = serializer.deserialize(
           propertyMapper,

diff --git a/sdk/core/core-client/test/serializer.spec.ts b/sdk/core/core-client/test/serializer.spec.ts
@@ -985,6 +985,45 @@ describe("Serializer", function() {
       }
     });
 
+    it("should correctly deserialize a pageable type with nextLink  first in mapper", function() {
+      const serializer = createSerializer(Mappers);
+      const mapper = Mappers.ProductListResultNextLinkFirst;
+      const responseBody = {
+        value: [
+          {
+            id: 101,
+            name: "TestProduct",
+            properties: {
+              provisioningState: "Succeeded"
+            }
+          },
+          {
+            id: 104,
+            name: "TestProduct1",
+            properties: {
+              provisioningState: "Failed"
+            }
+          }
+        ],
+        nextLink: "https://helloworld.com"
+      };
+      const deserializedProduct = serializer.deserialize(mapper, responseBody, "responseBody");
+      assert.isTrue(Array.isArray(deserializedProduct));
+      assert.equal(deserializedProduct.length, 2);
+      assert.equal(deserializedProduct.nextLink, "https://helloworld.com");
+      for (let i = 0; i < deserializedProduct.length; i++) {
+        if (i === 0) {
+          assert.equal(deserializedProduct[i].id, 101);
+          assert.equal(deserializedProduct[i].name, "TestProduct");
+          assert.equal(deserializedProduct[i].provisioningState, "Succeeded");
+        } else if (i === 1) {
+          assert.equal(deserializedProduct[i].id, 104);
+          assert.equal(deserializedProduct[i].name, "TestProduct1");
+          assert.equal(deserializedProduct[i].provisioningState, "Failed");
+        }
+      }
+    });
+
     it("should correctly deserialize a pageable type with nextLink", function() {
       const serializer = createSerializer(Mappers);
       const mapper = Mappers.ProductListResultNextLink;

diff --git a/sdk/core/core-client/test/testMappers.ts b/sdk/core/core-client/test/testMappers.ts
@@ -809,6 +809,36 @@ internalMappers.ProductListResultNextLink = {
     }
   }
 };
+internalMappers.ProductListResultNextLinkFirst = {
+  required: false,
+  serializedName: "ProductListResultNextLink",
+  type: {
+    name: "Composite",
+    className: "ProductListResultNextLink",
+    modelProperties: {
+      nextLink: {
+        serializedName: "nextLink",
+        required: false,
+        type: {
+          name: "String"
+        }
+      },
+      value: {
+        serializedName: "",
+        required: false,
+        type: {
+          name: "Sequence",
+          element: {
+            type: {
+              name: "Composite",
+              className: "Product"
+            }
+          }
+        }
+      }
+    }
+  }
+};
 internalMappers.SawShark = {
   required: false,
   serializedName: "sawshark",

diff --git a/sdk/core/core-http/CHANGELOG.md b/sdk/core/core-http/CHANGELOG.md
@@ -2,6 +2,9 @@
 
 ## 1.2.6 (Unreleased)
 
+### Key Bugs Fixed
+
+- Fixed an issue of lost properties when flattening array in deserialization [issue 15653](https://github.com/azure/azure-sdk-for-js/issues/15653)
 
 ## 1.2.5 (2021-06-03)
 

diff --git a/sdk/core/core-http/src/serializer.ts b/sdk/core/core-http/src/serializer.ts
@@ -936,12 +936,20 @@ function deserializeCompositeType(
       // paging
       if (Array.isArray(responseBody[key]) && modelProps[key].serializedName === "") {
         propertyInstance = responseBody[key];
-        instance = serializer.deserialize(
+        const arrayInstance = serializer.deserialize(
           propertyMapper,
           propertyInstance,
           propertyObjectName,
           options
         );
+        // Copy over any properties that have already been added into the instance, where they do
+        // not exist on the newly de-serialized array
+        for (const [k, v] of Object.entries(instance)) {
+          if (!Object.prototype.hasOwnProperty.call(arrayInstance, k)) {
+            arrayInstance[k] = v;
+          }
+        }
+        instance = arrayInstance;
       } else if (propertyInstance !== undefined || propertyMapper.defaultValue !== undefined) {
         serializedValue = serializer.deserialize(
           propertyMapper,

diff --git a/sdk/core/core-http/test/data/TestClient/src/models/mappers.ts b/sdk/core/core-http/test/data/TestClient/src/models/mappers.ts
@@ -546,6 +546,36 @@ internalMappers.ProductListResultNextLink = {
     }
   }
 };
+internalMappers.ProductListResultNextLinkFirst = {
+  required: false,
+  serializedName: "ProductListResultNextLink",
+  type: {
+    name: "Composite",
+    className: "ProductListResultNextLink",
+    modelProperties: {
+      nextLink: {
+        serializedName: "nextLink",
+        required: false,
+        type: {
+          name: "String"
+        }
+      },
+      value: {
+        serializedName: "",
+        required: false,
+        type: {
+          name: "Sequence",
+          element: {
+            type: {
+              name: "Composite",
+              className: "Product"
+            }
+          }
+        }
+      }
+    }
+  }
+};
 internalMappers.SawShark = {
   required: false,
   serializedName: "sawshark",

diff --git a/sdk/core/core-http/test/serializationTests.ts b/sdk/core/core-http/test/serializationTests.ts
@@ -1150,6 +1150,50 @@ describe("msrest", function() {
       done();
     });
 
+    it("should correctly deserialize a pageable type with nextLink first in mapper", function(done) {
+      const client = new TestClient("http://localhost:9090");
+      const mapper = Mappers.ProductListResultNextLinkFirst;
+      const responseBody = {
+        value: [
+          {
+            id: 101,
+            name: "TestProduct",
+            properties: {
+              provisioningState: "Succeeded"
+            }
+          },
+          {
+            id: 104,
+            name: "TestProduct1",
+            properties: {
+              provisioningState: "Failed"
+            }
+          }
+        ],
+        nextLink: "https://helloworld.com"
+      };
+      const deserializedProduct = client.serializer.deserialize(
+        mapper,
+        responseBody,
+        "responseBody"
+      );
+      Array.isArray(deserializedProduct).should.be.true;
+      deserializedProduct.length.should.equal(2);
+      deserializedProduct.nextLink.should.equal("https://helloworld.com");
+      for (let i = 0; i < deserializedProduct.length; i++) {
+        if (i === 0) {
+          deserializedProduct[i].id.should.equal(101);
+          deserializedProduct[i].name.should.equal("TestProduct");
+          deserializedProduct[i].provisioningState.should.equal("Succeeded");
+        } else if (i === 1) {
+          deserializedProduct[i].id.should.equal(104);
+          deserializedProduct[i].name.should.equal("TestProduct1");
+          deserializedProduct[i].provisioningState.should.equal("Failed");
+        }
+      }
+      done();
+    });
+
     it("should correctly deserialize object version of polymorphic discriminator", function(done) {
       const client = new TestClient("http://localhost:9090");
       const mapper = Mappers.Fish;