From 7c472821e575beede9431ecb315c7870e2ec9480 Mon Sep 17 00:00:00 2001 From: Johannes Bader Date: Thu, 24 Aug 2017 15:07:16 -0700 Subject: [PATCH] [ClientRuntime] Fixed polymorphic deserialization (#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) --- .../Properties/launchSettings.json | 8 -- .../JsonSerializationTests.cs | 24 +++- .../PolymorphicJsonConverterTests.cs | 130 ++++++++++++++++++ .../ClientRuntime.Tests/Resources/Animal.cs | 3 + .../PolymorphicDeserializeJsonConverter.cs | 48 +++++-- .../Serialization/PolymorphicJsonConverter.cs | 2 +- tools/generate.cmd | 10 +- tools/generateMetadata.ps1 | 9 +- 8 files changed, 211 insertions(+), 23 deletions(-) delete mode 100644 src/SDKs/Network/Network.Tests/Properties/launchSettings.json create mode 100644 src/SdkCommon/ClientRuntime/ClientRuntime.Tests/PolymorphicJsonConverterTests.cs diff --git a/src/SDKs/Network/Network.Tests/Properties/launchSettings.json b/src/SDKs/Network/Network.Tests/Properties/launchSettings.json deleted file mode 100644 index 90794c78573da..0000000000000 --- a/src/SDKs/Network/Network.Tests/Properties/launchSettings.json +++ /dev/null @@ -1,8 +0,0 @@ -{ - "profiles": { - "test": { - "commandName": "test", - "sdkVersion": "dnx-coreclr-win-x64.1.0.0-rc1-update1" - } - } -} diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/JsonSerializationTests.cs b/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/JsonSerializationTests.cs index e1534dbdefed2..09763fa545110 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/JsonSerializationTests.cs +++ b/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("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); } diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/PolymorphicJsonConverterTests.cs b/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/PolymorphicJsonConverterTests.cs new file mode 100644 index 0000000000000..53d0d86d34290 --- /dev/null +++ b/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("type"); + private static JsonConverter DeserializeJsonConverter => new PolymorphicDeserializeJsonConverter("type"); + + // helpers + + private static T RoundTrip(Model instance) + { + var json = JsonConvert.SerializeObject(instance, SerializeJsonConverter); + return JsonConvert.DeserializeObject(json, DeserializeJsonConverter); + } + + private static void AssertRoundTrips(T instance) where T : Model + { + var typeStatic = typeof(T); + var typeDynamicIn = instance.GetType(); + var typeDynamicOut = RoundTrip(instance).GetType(); + Assert.True(typeDynamicIn == typeDynamicOut, $"Round-tripping a '{typeStatic.Name}' unexpectedly failed: '{typeDynamicIn.Name}' -> '{typeDynamicOut.Name}'"); + } + + private static void AssertRoundTripFails(Model instance) + { + var typeStatic = typeof(T); + var typeDynamicIn = instance.GetType(); + var typeDynamicOut = RoundTrip(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(new Model()); + AssertRoundTripFails(new ModelX()); + AssertRoundTripFails(new ModelXA()); + AssertRoundTripFails(new ModelXB()); + AssertRoundTripFails(new ModelXAA()); + AssertRoundTripFails(new ModelXAB()); + AssertRoundTripFails(new ModelXABA()); + AssertRoundTripFails(new ModelXABB()); + + AssertRoundTripFails(new Model()); + AssertRoundTrips(new ModelX()); + AssertRoundTrips(new ModelXA()); + AssertRoundTrips(new ModelXB()); + AssertRoundTrips(new ModelXAA()); + AssertRoundTrips(new ModelXAB()); + AssertRoundTrips(new ModelXABA()); + AssertRoundTrips(new ModelXABB()); + + AssertRoundTripFails(new Model()); + AssertRoundTripFails(new ModelX()); + AssertRoundTrips(new ModelXA()); + AssertRoundTripFails(new ModelXB()); + AssertRoundTrips(new ModelXAA()); + AssertRoundTrips(new ModelXAB()); + AssertRoundTrips(new ModelXABA()); + AssertRoundTrips(new ModelXABB()); + + AssertRoundTripFails(new Model()); + AssertRoundTripFails(new ModelX()); + AssertRoundTripFails(new ModelXA()); + AssertRoundTrips(new ModelXB()); + AssertRoundTripFails(new ModelXAA()); + AssertRoundTripFails(new ModelXAB()); + AssertRoundTripFails(new ModelXABA()); + AssertRoundTripFails(new ModelXABB()); + + AssertRoundTripFails(new Model()); + AssertRoundTripFails(new ModelX()); + AssertRoundTripFails(new ModelXA()); + AssertRoundTripFails(new ModelXB()); + AssertRoundTrips(new ModelXAA()); + AssertRoundTripFails(new ModelXAB()); + AssertRoundTripFails(new ModelXABA()); + AssertRoundTripFails(new ModelXABB()); + + AssertRoundTripFails(new Model()); + AssertRoundTripFails(new ModelX()); + AssertRoundTripFails(new ModelXA()); + AssertRoundTripFails(new ModelXB()); + AssertRoundTripFails(new ModelXAA()); + AssertRoundTrips(new ModelXAB()); + AssertRoundTrips(new ModelXABA()); + AssertRoundTrips(new ModelXABB()); + + AssertRoundTripFails(new Model()); + AssertRoundTripFails(new ModelX()); + AssertRoundTripFails(new ModelXA()); + AssertRoundTripFails(new ModelXB()); + AssertRoundTripFails(new ModelXAA()); + AssertRoundTripFails(new ModelXAB()); + AssertRoundTrips(new ModelXABA()); + AssertRoundTripFails(new ModelXABB()); + + AssertRoundTripFails(new Model()); + AssertRoundTripFails(new ModelX()); + AssertRoundTripFails(new ModelXA()); + AssertRoundTripFails(new ModelXB()); + AssertRoundTripFails(new ModelXAA()); + AssertRoundTripFails(new ModelXAB()); + AssertRoundTripFails(new ModelXABA()); + AssertRoundTrips(new ModelXABB()); + } + } +} diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/Resources/Animal.cs b/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/Resources/Animal.cs index 153ec5de9a0f2..cce06ccebb99f 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime.Tests/Resources/Animal.cs +++ b/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; } } diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicDeserializeJsonConverter.cs b/src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicDeserializeJsonConverter.cs index 1c05da20a3f44..050b3d180f569 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicDeserializeJsonConverter.cs +++ b/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 } /// - /// 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. /// /// The type of the object to check. - /// True if the object being deserialized is the base type. False otherwise. + /// True if the object being deserialized is assignable to the base type. False otherwise. public override bool CanConvert(Type objectType) { - return typeof (T) == objectType; + return typeof(T).GetTypeInfo().IsAssignableFrom(objectType.GetTypeInfo()); + } + + /// + /// Case insensitive (and reduced version) of JToken.SelectToken which unfortunately does not offer + /// such functionality and has made all potential extension points `internal`. + /// + 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; } /// @@ -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; } /// diff --git a/src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicJsonConverter.cs b/src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicJsonConverter.cs index 57f46d6024c85..033a7c4da7693 100644 --- a/src/SdkCommon/ClientRuntime/ClientRuntime/Serialization/PolymorphicJsonConverter.cs +++ b/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().Any()) diff --git a/tools/generate.cmd b/tools/generate.cmd index a4fd546dfdd8c..e30cce1b43863 100644 --- a/tools/generate.cmd +++ b/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 ^ + echo ^ echo ^ echo ^ - echo ^ + echo ^ 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 diff --git a/tools/generateMetadata.ps1 b/tools/generateMetadata.ps1 index 751c778ae2397..f9fcddab92b3e 100644 --- a/tools/generateMetadata.ps1 +++ b/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"