INTERNAL Range: Fixes Range type serialization to not allow NOT supported combinations (#4680)

* Range type serialization assumption to not allow NOT suported combinations

* Fixing incorrect conflict resolution

* Addressing the PR comments

Author: kirankumarkolli

Microsoft.Azure.Cosmos/src/Routing/RangeJsonConverter.cs

@@ -7,12 +7,13 @@ namespace Microsoft.Azure.Cosmos.Routing
     using System;
     using Newtonsoft.Json;
     using Newtonsoft.Json.Linq;
-    using PartitionKeyRange = Documents.PartitionKeyRange;
 
     internal sealed class RangeJsonConverter : JsonConverter
     {
         private static readonly string MinProperty = "min";
         private static readonly string MaxProperty = "max";
+        private static readonly string MinInclusiveProperty = "isMinInclusive";
+        private static readonly string MaxInclusiveProperty = "isMaxInclusive";
 
         public override void WriteJson(JsonWriter writer, object value, JsonSerializer serializer)
         {
@@ -25,6 +26,17 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s
                 serializer.Serialize(writer, range.Min);
                 writer.WritePropertyName(MaxProperty);
                 serializer.Serialize(writer, range.Max);
+                if (!range.IsMinInclusive)
+                {
+                    writer.WritePropertyName(MinInclusiveProperty);
+                    writer.WriteValue(false);
+                }
+                if (range.IsMaxInclusive)
+                {
+                    writer.WritePropertyName(MaxInclusiveProperty);
+                    writer.WriteValue(true);
+                }
+
                 writer.WriteEndObject();
             }
             catch (Exception ex)
@@ -38,10 +50,23 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
             try
             {
                 JObject jsonObject = JObject.Load(reader);
+                bool isMinInclusive = true;
+                if (jsonObject.TryGetValue(MinInclusiveProperty, out JToken minInclusiveToken))
+                {
+                    isMinInclusive = (bool)minInclusiveToken;
+                }
+
+                bool isMaxInclusive = false;
+                if (jsonObject.TryGetValue(MaxInclusiveProperty, out JToken maxInclusiveToken))
+                {
+                    isMaxInclusive = (bool)maxInclusiveToken;
+                }
+
                 return new Documents.Routing.Range<string>(
                     jsonObject[MinProperty].Value<string>(),
                     jsonObject[MaxProperty].Value<string>(),
-                    true, false);
+                    isMinInclusive,
+                    isMaxInclusive);
             }
             catch (Exception ex)
             {
@@ -51,7 +76,7 @@ public override object ReadJson(JsonReader reader, Type objectType, object exist
 
         public override bool CanConvert(Type objectType)
         {
-            return typeof(PartitionKeyRange).IsAssignableFrom(objectType);
+            return typeof(Documents.Routing.Range<string>).IsAssignableFrom(objectType);
         }
     }
 }

Microsoft.Azure.Cosmos/tests/Microsoft.Azure.Cosmos.Tests/FeedRange/FeedRangeTests.cs

@@ -14,8 +14,11 @@ namespace Microsoft.Azure.Cosmos.Tests.FeedRange
     using Microsoft.Azure.Cosmos.Routing;
     using Moq;
     using Microsoft.Azure.Cosmos.Tracing;
-    using System.Net.Http;
+    using Newtonsoft.Json;
     using System.Text;
+    using System.IO;
+    using System.Net.Http;
+    using Newtonsoft.Json.Linq;
 
     [TestClass]
     public class FeedRangeTests
@@ -275,5 +278,94 @@ public async Task GetFeedRangesThrowsCosmosException()
                 Assert.IsTrue(invokedPkRanges);
             }
         }
+
+        /// <summary>
+        /// RangeJsonConverter accepts only (minInclusive=True, maxInclusive=False) combination
+        ///     In its serialization its not including minInclusive, maxInclusive combination 
+        ///     but on deserialization setting them to (true, false
+        ///     
+        /// All other combinations should throw an exception
+        /// </summary>
+        [TestMethod]
+        [DataRow(false, true)]
+        [DataRow(false, false)]
+        [DataRow(true, true)]
+        [DataRow(true, false)]
+        [Owner("kirankk")]
+        public void FeedRangeEpk_SerializationValidation(bool minInclusive, bool maxInclusive)
+        {
+            Documents.Routing.Range<string> range = new Documents.Routing.Range<string>("", "FF", minInclusive, maxInclusive);
+            RangeJsonConverter rangeConverter = new RangeJsonConverter();
+
+            using StringWriter sw = new StringWriter();
+            using JsonWriter writer = new JsonTextWriter(sw);
+            {
+                JsonSerializer jsonSerializer = new JsonSerializer();
+                rangeConverter.WriteJson(writer, range, jsonSerializer);
+                writer.Flush();
+                sw.Flush();
+
+                JObject parsedJson = JObject.Parse(sw.ToString());
+                Assert.AreEqual(true, parsedJson.ContainsKey("min"));
+                Assert.AreEqual(string.Empty, parsedJson["min"]);
+                Assert.AreEqual(true, parsedJson.ContainsKey("max"));
+                Assert.AreEqual("FF", parsedJson["max"]);
+                Assert.AreEqual(!minInclusive, parsedJson.ContainsKey("isMinInclusive"));
+                Assert.AreEqual(maxInclusive, parsedJson.ContainsKey("isMaxInclusive"));
+                if (!minInclusive)
+                {
+                    Assert.AreEqual(false, parsedJson["isMinInclusive"]);
+                }
+
+                if (maxInclusive)
+                {
+                    Assert.AreEqual(true, parsedJson["isMaxInclusive"]);
+                }
+            }
+        }
+
+        [TestMethod]
+        [DataRow(false, true)]
+        [DataRow(false, false)]
+        [DataRow(true, true)]
+        [DataRow(true, false)]
+        [Owner("kirankk")]
+        public void FeedRangeEpk_SerdeValdation(bool minInclusive, bool maxInclusive)
+        {
+            Documents.Routing.Range<string> range = new Documents.Routing.Range<string>("", "FF", minInclusive, maxInclusive);
+            RangeJsonConverter rangeConverter = new RangeJsonConverter();
+
+            using StringWriter sw = new StringWriter();
+            using JsonWriter writer = new JsonTextWriter(sw);
+            {
+                JsonSerializer jsonSerializer = new JsonSerializer();
+
+                rangeConverter.WriteJson(writer, range, jsonSerializer);
+
+                string serializedJson = sw.ToString();
+                System.Diagnostics.Trace.TraceInformation(serializedJson);
+
+                using TextReader reader = new StringReader(serializedJson);
+                using JsonReader jsonReader = new JsonTextReader(reader);
+                Documents.Routing.Range<string> rangeDeserialized = (Documents.Routing.Range<string>)rangeConverter.ReadJson(jsonReader, typeof(Documents.Routing.Range<string>), null, jsonSerializer);
+                Assert.IsTrue(range.Equals(rangeDeserialized), serializedJson);
+            }
+        }
+
+        [TestMethod]
+        [Owner("kirankk")]
+        public void FeedRangeEpk_BackwardComptibility()
+        {
+            string testJson = @"{""min"":"""",""max"":""FF""}";
+            System.Diagnostics.Trace.TraceInformation(testJson);
+            RangeJsonConverter rangeConverter = new RangeJsonConverter();
+
+            using TextReader reader = new StringReader(testJson);
+            using JsonReader jsonReader = new JsonTextReader(reader);
+            Documents.Routing.Range<string> rangeDeserialized = (Documents.Routing.Range<string>)rangeConverter.ReadJson(jsonReader, typeof(Documents.Routing.Range<string>), null, new JsonSerializer());
+
+            Assert.IsTrue(rangeDeserialized.IsMinInclusive);
+            Assert.IsFalse(rangeDeserialized.IsMaxInclusive);
+        }
     }
 }