Merge pull request #718 from Dreamescaper/support_existing_discrimina…

…nt_property

Allow existing string property to be discriminant

src/NJsonSchema.Tests/Generation/InheritanceTests.cs

@@ -1,4 +1,5 @@
-using System.Linq;
+using System;
+using System.Linq;
 using System.Runtime.Serialization;
 using System.Threading.Tasks;
 using Newtonsoft.Json;
@@ -254,5 +255,53 @@ public async Task When_deserializing_object_with_inheritance_then_correct_type_i
             /// Assert
             Assert.Equal(typeof(ACommonThing), vm.CommonThing.GetType());
         }
+
+        [KnownType(typeof(InheritedClass_WithStringDiscriminant))]
+        [JsonConverter(typeof(JsonInheritanceConverter), nameof(Kind))]
+        public class BaseClass_WithStringDiscriminant
+        {
+            public string Kind { get; set; }
+        }
+
+        public class InheritedClass_WithStringDiscriminant : BaseClass_WithStringDiscriminant
+        {
+
+        }
+
+        [Fact]
+        public async Task Existing_string_property_can_be_discriminant()
+        {
+            //// Arrange
+
+            //// Act
+            var schema = await JsonSchema4.FromTypeAsync<BaseClass_WithStringDiscriminant>();
+
+            //// Assert
+            Assert.NotNull(schema.Properties["Kind"]);
+        }
+
+        [KnownType(typeof(InheritedClass_WithIntDiscriminant))]
+        [JsonConverter(typeof(JsonInheritanceConverter), nameof(Kind))]
+        public class BaseClass_WithIntDiscriminant
+        {
+            public int Kind { get; set; }
+        }
+
+        public class InheritedClass_WithIntDiscriminant : BaseClass_WithStringDiscriminant
+        {
+
+        }
+
+        [Fact]
+        public async Task Existing_non_string_property_cant_be_discriminant()
+        {
+            //// Arrange
+
+            //// Act
+            Task<JsonSchema4> getSchema() => JsonSchema4.FromTypeAsync<BaseClass_WithIntDiscriminant>();
+
+            //// Assert
+            await Assert.ThrowsAsync<InvalidOperationException>(getSchema);
+        }
     }
 }

src/NJsonSchema.Tests/Generation/JsonInheritanceConverterTests.cs

@@ -0,0 +1,104 @@
+using System.IO;
+using System.Runtime.Serialization;
+using Newtonsoft.Json;
+using NJsonSchema.Converters;
+using Xunit;
+
+namespace NJsonSchema.Tests.Generation
+{
+    public class JsonInheritanceConverterTests
+    {
+        private static readonly JsonSerializer DefaultSerializer = JsonSerializer.CreateDefault();
+
+        [KnownType(typeof(ClassA))]
+        public class BaseClass
+        {
+            public string PropertyA { get; set; } = "defaultA";
+        }
+
+        public class ClassA : BaseClass
+        {
+            public string PropertyB { get; set; } = "defaultB";
+        }
+
+        [Fact]
+        public void When_serializing_discriminator_property_is_set()
+        {
+            // Arrange
+            var objA = new ClassA();
+            var stringWriter = new StringWriter();
+            var textWriter = new JsonTextWriter(stringWriter);
+
+            // Act
+            new JsonInheritanceConverter("discriminator").WriteJson(textWriter, objA, DefaultSerializer);
+
+            // Assert
+            var json = stringWriter.ToString();
+            Assert.Contains("\"discriminator\":\"ClassA\"", json);
+            Assert.Contains("\"PropertyA\":\"defaultA\"", json);
+            Assert.Contains("\"PropertyB\":\"defaultB\"", json);
+        }
+
+        [Fact]
+        public void When_serializing_discriminator_property_is_overwritten_if_already_present()
+        {
+            // Arrange
+            var objA = new ClassA();
+            var stringWriter = new StringWriter();
+            var jsonWriter = new JsonTextWriter(stringWriter);
+
+            // Act
+            new JsonInheritanceConverter("PropertyA").WriteJson(jsonWriter, objA, DefaultSerializer);
+
+            // Assert
+            var json = stringWriter.ToString();
+            Assert.Contains("\"PropertyA\":\"ClassA\"", json);
+            Assert.Contains("\"PropertyB\":\"defaultB\"", json);
+        }
+
+        [Fact]
+        public void When_deserializing_type_is_resolved_using_discriminator_value()
+        {
+            // Arrange
+            var json = @"
+                {
+                    ""PropertyA"":""v1"",
+                    ""PropertyB"":""v2"",
+                    ""discriminator"":""ClassA""
+                }";
+            var jsonReader = new JsonTextReader(new StringReader(json));
+
+            // Act
+            var obj = new JsonInheritanceConverter("discriminator").ReadJson(jsonReader, typeof(BaseClass), null, DefaultSerializer);
+
+            // Assert
+            Assert.IsType<ClassA>(obj);
+
+            var objA = (ClassA)obj;
+            Assert.Equal("v1", objA.PropertyA);
+            Assert.Equal("v2", objA.PropertyB);
+        }
+
+        [Fact]
+        public void When_deserializing_existing_property_is_populated_with_discriminator_value()
+        {
+            // Arrange
+            var json = @"
+                {
+                    ""PropertyA"":""ClassA"",
+                    ""PropertyB"":""v2""
+                }";
+            var jsonReader = new JsonTextReader(new StringReader(json));
+
+            // Act
+            var obj = new JsonInheritanceConverter("PropertyA").ReadJson(jsonReader, typeof(BaseClass), null, DefaultSerializer);
+
+            // Assert
+            Assert.IsType<ClassA>(obj);
+
+            var objA = (ClassA)obj;
+            Assert.Equal("ClassA", objA.PropertyA);
+            Assert.Equal("v2", objA.PropertyB);
+        }
+    }
+}

src/NJsonSchema/Converters/JsonInheritanceConverter.cs

@@ -86,7 +86,7 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s
                 _isWriting = true;
 
                 var jObject = JObject.FromObject(value, serializer);
-                jObject.AddFirst(new JProperty(_discriminator, GetDiscriminatorValue(value.GetType())));
+                jObject[_discriminator] = JToken.FromObject(GetDiscriminatorValue(value.GetType()));
                 writer.WriteToken(jObject.CreateReader());
             }
             finally

src/NJsonSchema/Generation/JsonSchemaGenerator.cs

@@ -6,6 +6,12 @@
 // <author>Rico Suter, mail@rsuter.com</author>
 //-----------------------------------------------------------------------
 
+using System;
+using System.Collections;
+using System.Collections.Generic;
+using System.Linq;
+using System.Reflection;
+using System.Threading.Tasks;
 using Newtonsoft.Json;
 using Newtonsoft.Json.Converters;
 using Newtonsoft.Json.Linq;
@@ -14,12 +20,6 @@
 using NJsonSchema.Converters;
 using NJsonSchema.Generation.TypeMappers;
 using NJsonSchema.Infrastructure;
-using System;
-using System.Collections;
-using System.Collections.Generic;
-using System.Linq;
-using System.Reflection;
-using System.Threading.Tasks;
 
 namespace NJsonSchema.Generation
 {
@@ -686,8 +686,13 @@ private void GenerateInheritanceDiscriminator(Type type, JsonSchema4 schema)
                 if (discriminatorConverter != null)
                 {
                     var discriminatorName = TryGetInheritanceDiscriminatorName(discriminatorConverter);
-                    if (schema.Properties.ContainsKey(discriminatorName))
-                        throw new InvalidOperationException("The JSON property '" + discriminatorName + "' is defined multiple times on type '" + type.FullName + "'.");
+
+                    // Existing property can be discriminator only if it has String type  
+                    if (schema.Properties.TryGetValue(discriminatorName, out JsonProperty existingProperty) && 
+                        (existingProperty.Type & JsonObjectType.String) == 0)
+                    {
+                        throw new InvalidOperationException("The JSON discriminator property '" + discriminatorName + "' must be a string property on type '" + type.FullName + "' (it is recommended to not implement the discriminator property at all).");
+                    }
 
                     var discriminator = new OpenApiDiscriminator
                     {