[ClientRuntime] Fixed polymorphic deserialization (Azure#3503)

* reproing test (deep hiararchy)

* reproing test (base class property)

* fix

* bold move

* tiny improvement to generate.cmd (just making sure you get the same experience even when calling one of the RPs scripts)

* case insensitive property lookup (I guess we're already down that rabbit hole)

* tiny improvement to generate.cmd (mention commit IDs, clarifications)

* tiny improvement to generateMetadata.cmd (handle commit IDs)

src/SdkCommon/ClientRuntime/ClientRuntime.Tests/JsonSerializationTests.cs

@@ -25,8 +25,26 @@ public class JsonSerializationTests
         public void PolymorphicSerializeWorks()
         {
             Zoo zoo = new Zoo() { Id = 1 };
-            zoo.Animals.Add(new Dog() { Name = "Fido", LikesDogfood = true });
-            zoo.Animals.Add(new Cat() { Name = "Felix", LikesMice = false, Dislikes = new Dog() { Name = "Angry", LikesDogfood = true } });
+            zoo.Animals.Add(new Dog() {
+                Name = "Fido",
+                LikesDogfood = true
+            });
+            zoo.Animals.Add(new Cat() {
+                Name = "Felix",
+                LikesMice = false,
+                Dislikes = new Dog() {
+                    Name = "Angry",
+                    LikesDogfood = true
+                },
+                BestFriend = new Animal() {
+                    Name = "Rudy the Rabbit",
+                    BestFriend = new Cat()
+                    {
+                        Name = "Jango",
+                        LikesMice = true
+                    }
+                }
+            });
             var serializeSettings = new JsonSerializerSettings();
             serializeSettings.ReferenceLoopHandling = ReferenceLoopHandling.Serialize;
             serializeSettings.Converters.Add(new PolymorphicSerializeJsonConverter<Animal>("dType"));
@@ -41,6 +59,8 @@ public void PolymorphicSerializeWorks()
             Assert.Equal(zoo.Animals[0].GetType(), zoo2.Animals[0].GetType());
             Assert.Equal(zoo.Animals[1].GetType(), zoo2.Animals[1].GetType());
             Assert.Equal(((Cat)zoo.Animals[1]).Dislikes.GetType(), ((Cat)zoo2.Animals[1]).Dislikes.GetType());
+            Assert.Equal(zoo.Animals[1].BestFriend.GetType(), zoo2.Animals[1].BestFriend.GetType());
+            Assert.Equal(zoo.Animals[1].BestFriend.BestFriend.GetType(), zoo2.Animals[1].BestFriend.BestFriend.GetType());
             Assert.Contains("dType", serializedJson);
         }
 

src/SdkCommon/ClientRuntime/ClientRuntime.Tests/PolymorphicJsonConverterTests.cs

@@ -0,0 +1,130 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+using Xunit;
+using Newtonsoft.Json;
+using Microsoft.Rest.Serialization;
+
+namespace Microsoft.Rest.ClientRuntime.Tests
+{
+    public class PolymorphicJsonConverterTests
+    {
+        // Example hierarchy
+        // Naming convention:  nameof(U).StartsWith(nameof(T))  <=>  U : T
+
+        private class Model { }
+        private class ModelX : Model { }
+        private class ModelXA : ModelX { }
+        private class ModelXB : ModelX { }
+        private class ModelXAA : ModelXA { }
+        private class ModelXAB : ModelXA { }
+        private class ModelXABA : ModelXAB { }
+        private class ModelXABB : ModelXAB { }
+
+        // JsonConverters
+
+        private static JsonConverter SerializeJsonConverter => new PolymorphicSerializeJsonConverter<ModelX>("type");
+        private static JsonConverter DeserializeJsonConverter => new PolymorphicDeserializeJsonConverter<ModelX>("type");
+
+        // helpers
+
+        private static T RoundTrip<T>(Model instance)
+        {
+            var json = JsonConvert.SerializeObject(instance, SerializeJsonConverter);
+            return JsonConvert.DeserializeObject<T>(json, DeserializeJsonConverter);
+        }
+
+        private static void AssertRoundTrips<T>(T instance) where T : Model
+        {
+            var typeStatic = typeof(T);
+            var typeDynamicIn = instance.GetType();
+            var typeDynamicOut = RoundTrip<T>(instance).GetType();
+            Assert.True(typeDynamicIn == typeDynamicOut, $"Round-tripping a '{typeStatic.Name}' unexpectedly failed: '{typeDynamicIn.Name}' -> '{typeDynamicOut.Name}'");
+        }
+
+        private static void AssertRoundTripFails<T>(Model instance)
+        {
+            var typeStatic = typeof(T);
+            var typeDynamicIn = instance.GetType();
+            var typeDynamicOut = RoundTrip<T>(instance).GetType();
+            Assert.True(typeDynamicIn != typeDynamicOut, $"Round-tripping a '{typeStatic.Name}' unexpectedly succeeded: '{typeDynamicIn.Name}' -> '{typeDynamicOut.Name}'");
+        }
+
+        [Fact]
+        public void RoundTripping()
+        {
+            // Note: only Model itself succeeds since we put the discriminator on ModelX
+            AssertRoundTrips<Model>(new Model());
+            AssertRoundTripFails<Model>(new ModelX());
+            AssertRoundTripFails<Model>(new ModelXA());
+            AssertRoundTripFails<Model>(new ModelXB());
+            AssertRoundTripFails<Model>(new ModelXAA());
+            AssertRoundTripFails<Model>(new ModelXAB());
+            AssertRoundTripFails<Model>(new ModelXABA());
+            AssertRoundTripFails<Model>(new ModelXABB());
+
+            AssertRoundTripFails<ModelX>(new Model());
+            AssertRoundTrips<ModelX>(new ModelX());
+            AssertRoundTrips<ModelX>(new ModelXA());
+            AssertRoundTrips<ModelX>(new ModelXB());
+            AssertRoundTrips<ModelX>(new ModelXAA());
+            AssertRoundTrips<ModelX>(new ModelXAB());
+            AssertRoundTrips<ModelX>(new ModelXABA());
+            AssertRoundTrips<ModelX>(new ModelXABB());
+
+            AssertRoundTripFails<ModelXA>(new Model());
+            AssertRoundTripFails<ModelXA>(new ModelX());
+            AssertRoundTrips<ModelXA>(new ModelXA());
+            AssertRoundTripFails<ModelXA>(new ModelXB());
+            AssertRoundTrips<ModelXA>(new ModelXAA());
+            AssertRoundTrips<ModelXA>(new ModelXAB());
+            AssertRoundTrips<ModelXA>(new ModelXABA());
+            AssertRoundTrips<ModelXA>(new ModelXABB());
+
+            AssertRoundTripFails<ModelXB>(new Model());
+            AssertRoundTripFails<ModelXB>(new ModelX());
+            AssertRoundTripFails<ModelXB>(new ModelXA());
+            AssertRoundTrips<ModelXB>(new ModelXB());
+            AssertRoundTripFails<ModelXB>(new ModelXAA());
+            AssertRoundTripFails<ModelXB>(new ModelXAB());
+            AssertRoundTripFails<ModelXB>(new ModelXABA());
+            AssertRoundTripFails<ModelXB>(new ModelXABB());
+
+            AssertRoundTripFails<ModelXAA>(new Model());
+            AssertRoundTripFails<ModelXAA>(new ModelX());
+            AssertRoundTripFails<ModelXAA>(new ModelXA());
+            AssertRoundTripFails<ModelXAA>(new ModelXB());
+            AssertRoundTrips<ModelXAA>(new ModelXAA());
+            AssertRoundTripFails<ModelXAA>(new ModelXAB());
+            AssertRoundTripFails<ModelXAA>(new ModelXABA());
+            AssertRoundTripFails<ModelXAA>(new ModelXABB());
+
+            AssertRoundTripFails<ModelXAB>(new Model());
+            AssertRoundTripFails<ModelXAB>(new ModelX());
+            AssertRoundTripFails<ModelXAB>(new ModelXA());
+            AssertRoundTripFails<ModelXAB>(new ModelXB());
+            AssertRoundTripFails<ModelXAB>(new ModelXAA());
+            AssertRoundTrips<ModelXAB>(new ModelXAB());
+            AssertRoundTrips<ModelXAB>(new ModelXABA());
+            AssertRoundTrips<ModelXAB>(new ModelXABB());
+
+            AssertRoundTripFails<ModelXABA>(new Model());
+            AssertRoundTripFails<ModelXABA>(new ModelX());
+            AssertRoundTripFails<ModelXABA>(new ModelXA());
+            AssertRoundTripFails<ModelXABA>(new ModelXB());
+            AssertRoundTripFails<ModelXABA>(new ModelXAA());
+            AssertRoundTripFails<ModelXABA>(new ModelXAB());
+            AssertRoundTrips<ModelXABA>(new ModelXABA());
+            AssertRoundTripFails<ModelXABA>(new ModelXABB());
+
+            AssertRoundTripFails<ModelXABB>(new Model());
+            AssertRoundTripFails<ModelXABB>(new ModelX());
+            AssertRoundTripFails<ModelXABB>(new ModelXA());
+            AssertRoundTripFails<ModelXABB>(new ModelXB());
+            AssertRoundTripFails<ModelXABB>(new ModelXAA());
+            AssertRoundTripFails<ModelXABB>(new ModelXAB());
+            AssertRoundTripFails<ModelXABB>(new ModelXABA());
+            AssertRoundTrips<ModelXABB>(new ModelXABB());
+        }
+    }
+}

src/SdkCommon/ClientRuntime/ClientRuntime.Tests/Resources/Animal.cs

@@ -9,6 +9,9 @@ namespace Microsoft.Rest.ClientRuntime.Tests.Resources
     [JsonObject("animal")]
     public class Animal
     {
+        [JsonProperty("bestFriend")]
+        public Animal BestFriend { get; set; }
+
         [JsonProperty("name")]
         public string Name { get; set; }
     }

src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicDeserializeJsonConverter.cs

@@ -2,8 +2,11 @@
 // Licensed under the MIT License. See License.txt in the project root for license information.
 
 using System;
+using System.Reflection;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Linq;
+using Newtonsoft.Json.Serialization;
+using System.Linq;
 
 namespace Microsoft.Rest.Serialization
 {
@@ -36,13 +39,30 @@ public override bool CanWrite
         }
 
         /// <summary>
-        /// Returns true if the object being deserialized is the base type. False otherwise.
+        /// Returns true if the object being deserialized is assignable to the base type. False otherwise.
         /// </summary>
         /// <param name="objectType">The type of the object to check.</param>
-        /// <returns>True if the object being deserialized is the base type. False otherwise.</returns>
+        /// <returns>True if the object being deserialized is assignable to the base type. False otherwise.</returns>
         public override bool CanConvert(Type objectType)
         {
-            return typeof (T) == objectType;
+            return typeof(T).GetTypeInfo().IsAssignableFrom(objectType.GetTypeInfo());
+        }
+
+        /// <summary>
+        /// Case insensitive (and reduced version) of JToken.SelectToken which unfortunately does not offer
+        /// such functionality and has made all potential extension points `internal`.
+        /// </summary>
+        private JToken SelectTokenCaseInsensitive(JObject obj, string path)
+        {
+            JToken result = obj;
+            foreach (var pathComponent in path.Split('.'))
+            {
+                result = (result as JObject)?
+                    .Properties()
+                    .FirstOrDefault(p => string.Equals(p.Name, pathComponent, StringComparison.OrdinalIgnoreCase))?
+                    .Value;
+            }
+            return result;
         }
 
         /// <summary>
@@ -63,13 +83,25 @@ public override object ReadJson(JsonReader reader,
             }
 
             JObject item = JObject.Load(reader);
-            string typeDiscriminator = (string) item[Discriminator];
-            Type derivedType = GetDerivedType(typeof (T), typeDiscriminator);
-            if (derivedType != null)
+            string typeDiscriminator = (string)item[Discriminator];
+            Type resultType = GetDerivedType(objectType, typeDiscriminator) ?? objectType;
+
+            // create instance of correct type
+            var contract = (JsonObjectContract)serializer.ContractResolver.ResolveContract(resultType);
+            var result = contract.DefaultCreator();
+
+            // parse properties
+            foreach (var expectedProperty in contract.Properties)
             {
-                return item.ToObject(derivedType, serializer);
+                var property = SelectTokenCaseInsensitive(item, expectedProperty.PropertyName);
+                if (property != null)
+                {
+                    var propertyValue = property.ToObject(expectedProperty.PropertyType, serializer);
+                    expectedProperty.ValueProvider.SetValue(result, propertyValue);
+                }
             }
-            return item.ToObject(objectType);
+
+            return result;
         }
 
         /// <summary>

src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicJsonConverter.cs

@@ -31,7 +31,7 @@ public static Type GetDerivedType(Type baseType, string name)
                 throw new ArgumentNullException("baseType");
             }
             foreach (TypeInfo type in baseType.GetTypeInfo().Assembly.DefinedTypes
-                .Where(t => t.Namespace == baseType.Namespace && t != baseType.GetTypeInfo() && t.IsSubclassOf(baseType)))
+                .Where(t => t.Namespace == baseType.Namespace && baseType.GetTypeInfo().IsAssignableFrom(t)))
             {
                 string typeName = type.Name;
                 if (type.GetCustomAttributes<JsonObjectAttribute>().Any())

tools/generate.cmd

@@ -9,6 +9,10 @@ setlocal
 :: help requested?
 set pot_help=%1%2
 if "%pot_help%"==""            (set req_help=T)
+if "%2"=="help"                (set req_help=T)
+if "%2"=="-help"               (set req_help=T)
+if "%2"=="--help"              (set req_help=T)
+if "%2"=="/help"               (set req_help=T)
 if "%pot_help%"=="help"        (set req_help=T)
 if "%pot_help%"=="-help"       (set req_help=T)
 if "%pot_help%"=="--help"      (set req_help=T)
@@ -17,15 +21,15 @@ if not x%pot_help%==x%pot_help:?=% (set req_help=T)
 if not "%req_help%" == "" (
     echo.
     echo Usage: generate.cmd
-    echo             ^<RP, e.g. 'network/resource-manager'^>
+    echo             ^<service, e.g. 'network/resource-manager'^>
     echo             ^<AutoRest version, defaults to 'latest'^>
     echo             ^<GitHub user of azure-rest-api-specs repo, defaults to 'Azure'^>
-    echo             ^<Branch of azure-rest-api-specs repo, defaults to 'current'^>
+    echo             ^<Branch or commit ID of azure-rest-api-specs repo, defaults to 'current'^>
     echo.
     echo Example: generate.cmd monitor/data-plane 1.1.0 olydis new-cool-feature
+    echo Note: If you are calling an SDK's generate.cmd, the first parameter is already provided for you.
     echo.
     echo To display this help, run either of
-    echo      generate.cmd
     echo      generate.cmd help
     echo      generate.cmd -help
     echo      generate.cmd --help

tools/generateMetadata.ps1

@@ -4,7 +4,14 @@ Write-Host ""
 Write-Host "1) azure-rest-api-specs repository information"
 Write-Host "GitHub user:" $Args[0]
 Write-Host "Branch:     " $Args[1]
-Write-Host "Commit:     " (Invoke-RestMethod "https://api.github.com/repos/$($Args[0])/azure-rest-api-specs/branches/$($Args[1])").commit.sha
+Try
+{
+    Write-Host "Commit:     " (Invoke-RestMethod "https://api.github.com/repos/$($Args[0])/azure-rest-api-specs/branches/$($Args[1])").commit.sha
+}
+Catch
+{
+    # if the above REST call fails, a commit ID was passed, so we already got the information we need
+}
 
 Write-Host ""
 Write-Host "2) AutoRest information"