Add copy constructor to JsonSerializerOptions (#34725)
authorLayomi Akinrinade <laakinri@microsoft.com>
Fri, 10 Apr 2020 14:41:51 +0000 (10:41 -0400)
committerGitHub <noreply@github.com>
Fri, 10 Apr 2020 14:41:51 +0000 (07:41 -0700)
* Add copy constructor to JsonSerializerOptions

* Address review comments

* Address review feedback - null check and reflection based test

src/libraries/System.Text.Json/ref/System.Text.Json.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/ConverterList.cs
src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerOptions.cs
src/libraries/System.Text.Json/tests/Serialization/OptionsTests.cs

index 5c3e9d8..f206b3a 100644 (file)
@@ -208,6 +208,7 @@ namespace System.Text.Json
     public sealed partial class JsonSerializerOptions
     {
         public JsonSerializerOptions() { }
+        public JsonSerializerOptions(System.Text.Json.JsonSerializerOptions options) { }
         public bool AllowTrailingCommas { get { throw null; } set { } }
         public System.Collections.Generic.IList<System.Text.Json.Serialization.JsonConverter> Converters { get { throw null; } }
         public int DefaultBufferSize { get { throw null; } set { } }
index 8a39095..73a25b4 100644 (file)
@@ -20,6 +20,12 @@ namespace System.Text.Json.Serialization
             _options = options;
         }
 
+        public ConverterList(JsonSerializerOptions options, ConverterList source)
+        {
+            _options = options;
+            _list = new List<JsonConverter>(source._list);
+        }
+
         public JsonConverter this[int index]
         {
             get
index 8549f90..7d4c82b 100644 (file)
@@ -20,6 +20,8 @@ namespace System.Text.Json
 
         private readonly ConcurrentDictionary<Type, JsonClassInfo> _classes = new ConcurrentDictionary<Type, JsonClassInfo>();
 
+        // For any new option added, adding it to the options copied in the copy constructor below must be considered.
+
         private MemberAccessor? _memberAccessorStrategy;
         private JsonNamingPolicy? _dictionaryKeyPolicy;
         private JsonNamingPolicy? _jsonPropertyNamingPolicy;
@@ -45,6 +47,44 @@ namespace System.Text.Json
         }
 
         /// <summary>
+        /// Copies the options from a <see cref="JsonSerializerOptions"/> instance to a new instance.
+        /// </summary>
+        /// <param name="options">The <see cref="JsonSerializerOptions"/> instance to copy options from.</param>
+        /// <exception cref="System.ArgumentNullException">
+        /// <paramref name="options"/> is <see langword="null"/>.
+        /// </exception>
+        public JsonSerializerOptions(JsonSerializerOptions options)
+        {
+            if (options == null)
+            {
+                throw new ArgumentNullException(nameof(options));
+            }
+
+            _memberAccessorStrategy = options.MemberAccessorStrategy;
+            _dictionaryKeyPolicy = options._dictionaryKeyPolicy;
+            _jsonPropertyNamingPolicy = options._jsonPropertyNamingPolicy;
+            _readCommentHandling = options._readCommentHandling;
+            _referenceHandling = options._referenceHandling;
+            _encoder = options._encoder;
+
+            _defaultBufferSize = options._defaultBufferSize;
+            _maxDepth = options._maxDepth;
+            _allowTrailingCommas = options._allowTrailingCommas;
+            _ignoreNullValues = options._ignoreNullValues;
+            _ignoreReadOnlyProperties = options._ignoreReadOnlyProperties;
+            _propertyNameCaseInsensitive = options._propertyNameCaseInsensitive;
+            _writeIndented = options._writeIndented;
+
+            Converters = new ConverterList(this, (ConverterList)options.Converters);
+            EffectiveMaxDepth = options.EffectiveMaxDepth;
+
+            // _classes is not copied as sharing the JsonClassInfo and JsonPropertyInfo caches can result in
+            // unnecessary references to type metadata, potentially hindering garbage collection on the source options.
+
+            // _haveTypesBeenCreated is not copied; it's okay to make changes to this options instance as (de)serialization has not occured.
+        }
+
+        /// <summary>
         /// Defines whether an extra comma at the end of a list of JSON values in an object or array
         /// is allowed (and ignored) within the JSON payload being deserialized.
         /// </summary>
index b471421..aedd10b 100644 (file)
@@ -2,8 +2,10 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 // See the LICENSE file in the project root for more information.
 
+using System.Collections;
 using System.Collections.Generic;
 using System.IO;
+using System.Reflection;
 using System.Text.Encodings.Web;
 using System.Text.Unicode;
 using Xunit;
@@ -448,5 +450,233 @@ namespace System.Text.Json.Serialization.Tests
                 Assert.Equal(stringValue + stringValue, Encoding.UTF8.GetString(stream.ToArray()));
             }
         }
+
+        [Fact]
+        public static void CopyConstructorTest_OriginalMutable()
+        {
+            JsonSerializerOptions options = CreateOptionsInstance();
+            var newOptions = new JsonSerializerOptions(options);
+            VerifyOptionsEqual(options, newOptions);
+
+            // No exception is thrown on mutating the new options instance because it is "unlocked".
+            newOptions.ReferenceHandling = ReferenceHandling.Preserve;
+            newOptions.Converters.Add(new JsonStringEnumConverter());
+            newOptions.Converters.Add(new JsonStringEnumConverter());
+
+            // Changes to new options don't affect old options.
+            Assert.Equal(ReferenceHandling.Default, options.ReferenceHandling);
+            Assert.Equal(2, options.Converters.Count);
+            Assert.Equal(4, newOptions.Converters.Count);
+
+            // Changes to old options don't affect new options.
+            options.DefaultBufferSize = 2;
+            options.Converters.Add(new ConverterForInt32());
+
+            Assert.Equal(20, newOptions.DefaultBufferSize);
+            Assert.Equal(3, options.Converters.Count);
+            Assert.Equal(4, newOptions.Converters.Count);
+        }
+
+        [Fact]
+        public static void CopyConstructorTest_OriginalLocked()
+        {
+            JsonSerializerOptions options = CreateOptionsInstance();
+
+            // Perform serialization with options, after which it will be locked.
+            JsonSerializer.Serialize("1", options);
+            Assert.Throws<InvalidOperationException>(() => options.ReferenceHandling = ReferenceHandling.Preserve);
+
+            var newOptions = new JsonSerializerOptions(options);
+            VerifyOptionsEqual(options, newOptions);
+
+            // No exception is thrown on mutating the new options instance because it is "unlocked".
+            newOptions.ReferenceHandling = ReferenceHandling.Preserve;
+        }
+
+        [Fact]
+        public static void CopyConstructorTest_MaxDepth()
+        {
+            static void RunTest(int maxDepth, int effectiveMaxDepth)
+            {
+                var options = new JsonSerializerOptions { MaxDepth = maxDepth };
+                var newOptions = new JsonSerializerOptions(options);
+
+                Assert.Equal(maxDepth, options.MaxDepth);
+                Assert.Equal(maxDepth, newOptions.MaxDepth);
+
+                // Test for default effective max depth in exception message.
+                var myList = new List<object>();
+                myList.Add(myList);
+
+                string effectiveMaxDepthAsStr = effectiveMaxDepth.ToString();
+
+                JsonException ex = Assert.Throws<JsonException>(() => JsonSerializer.Serialize(myList, options));
+                Assert.Contains(effectiveMaxDepthAsStr, ex.ToString());
+
+                ex = Assert.Throws<JsonException>(() => JsonSerializer.Serialize(myList, newOptions));
+                Assert.Contains(effectiveMaxDepthAsStr, ex.ToString());
+            }
+
+            // Zero max depth
+            RunTest(0, 64);
+
+            // Specified max depth
+            RunTest(25, 25);
+        }
+
+        [Fact]
+        public static void CopyConstructorTest_CopiesAllPublicProperties()
+        {
+            JsonSerializerOptions options = GetFullyPopulatedOptionsInstance();
+            var newOptions = new JsonSerializerOptions(options);
+            VerifyOptionsEqual_WithReflection(options, newOptions);
+        }
+
+        [Fact]
+        public static void CopyConstructorTest_NullInput()
+        {
+            ArgumentNullException ex = Assert.Throws<ArgumentNullException>(() => new JsonSerializerOptions(null));
+            Assert.Contains("options", ex.ToString());
+        }
+
+        private static JsonSerializerOptions CreateOptionsInstance()
+        {
+            var options = new JsonSerializerOptions
+            {
+                AllowTrailingCommas = true,
+                DefaultBufferSize = 20,
+                DictionaryKeyPolicy = JsonNamingPolicy.CamelCase,
+                Encoder = JavaScriptEncoder.Default,
+                IgnoreNullValues = true,
+                IgnoreReadOnlyProperties = true,
+                MaxDepth = 32,
+                PropertyNameCaseInsensitive = true,
+                PropertyNamingPolicy = new SimpleSnakeCasePolicy(),
+                ReadCommentHandling = JsonCommentHandling.Disallow,
+                ReferenceHandling = ReferenceHandling.Default,
+                WriteIndented = true,
+            };
+
+            options.Converters.Add(new JsonStringEnumConverter());
+            options.Converters.Add(new ConverterForInt32());
+
+            return options;
+        }
+
+        private static void VerifyOptionsEqual(JsonSerializerOptions options, JsonSerializerOptions newOptions)
+        {
+            Assert.Equal(options.AllowTrailingCommas, newOptions.AllowTrailingCommas);
+            Assert.Equal(options.DefaultBufferSize, newOptions.DefaultBufferSize);
+            Assert.Equal(options.IgnoreNullValues, newOptions.IgnoreNullValues);
+            Assert.Equal(options.IgnoreReadOnlyProperties, newOptions.IgnoreReadOnlyProperties);
+            Assert.Equal(options.MaxDepth, newOptions.MaxDepth);
+            Assert.Equal(options.PropertyNameCaseInsensitive, newOptions.PropertyNameCaseInsensitive);
+            Assert.Equal(options.ReadCommentHandling, newOptions.ReadCommentHandling);
+            Assert.Equal(options.WriteIndented, newOptions.WriteIndented);
+
+            Assert.Same(options.DictionaryKeyPolicy, newOptions.DictionaryKeyPolicy);
+            Assert.Same(options.Encoder, newOptions.Encoder);
+            Assert.Same(options.PropertyNamingPolicy, newOptions.PropertyNamingPolicy);
+
+            Assert.Equal(options.Converters.Count, newOptions.Converters.Count);
+            for (int i = 0; i < options.Converters.Count; i++)
+            {
+                Assert.Same(options.Converters[i], newOptions.Converters[i]);
+            }
+        }
+
+        private static JsonSerializerOptions GetFullyPopulatedOptionsInstance()
+        {
+            var options = new JsonSerializerOptions();
+
+            foreach (PropertyInfo property in typeof(JsonSerializerOptions).GetProperties())
+            {
+                Type propertyType = property.PropertyType;
+
+                if (propertyType == typeof(bool))
+                {
+                    property.SetValue(options, true);
+                }
+                if (propertyType == typeof(int))
+                {
+                    property.SetValue(options, 32);
+                }
+                else if (propertyType == typeof(IList<JsonConverter>))
+                {
+                    options.Converters.Add(new JsonStringEnumConverter());
+                    options.Converters.Add(new ConverterForInt32());
+                }
+                else if (propertyType == typeof(JavaScriptEncoder))
+                {
+                    options.Encoder = JavaScriptEncoder.Default;
+                }
+                else if (propertyType == typeof(JsonNamingPolicy))
+                {
+                    options.PropertyNamingPolicy = JsonNamingPolicy.CamelCase;
+                    options.DictionaryKeyPolicy = new SimpleSnakeCasePolicy();
+                }
+                else if (propertyType == typeof(ReferenceHandling))
+                {
+                    options.ReferenceHandling = ReferenceHandling.Preserve;
+                }
+                else if (propertyType.IsValueType)
+                {
+                    options.ReadCommentHandling = JsonCommentHandling.Disallow;
+                }
+                else
+                {
+                    // An exception thrown from here means this test should be updated
+                    // to reflect any newly added properties on JsonSerializerOptions.
+                    property.SetValue(options, Activator.CreateInstance(propertyType));
+                }
+            }
+
+            return options;
+        }
+
+        private static void VerifyOptionsEqual_WithReflection(JsonSerializerOptions options, JsonSerializerOptions newOptions)
+        {
+            foreach (PropertyInfo property in typeof(JsonSerializerOptions).GetProperties())
+            {
+                Type propertyType = property.PropertyType;
+
+                if (propertyType == typeof(bool))
+                {
+                    Assert.True((bool)property.GetValue(options));
+                    Assert.Equal((bool)property.GetValue(options), (bool)property.GetValue(newOptions));
+                }
+                else if (propertyType == typeof(int))
+                {
+                    Assert.Equal(32, (int)property.GetValue(options));
+                    Assert.Equal((int)property.GetValue(options), (int)property.GetValue(newOptions));
+                }
+                else if (typeof(IEnumerable).IsAssignableFrom(propertyType))
+                {
+                    var list1 = (IList<JsonConverter>)property.GetValue(options);
+                    var list2 = (IList<JsonConverter>)property.GetValue(newOptions);
+
+                    Assert.Equal(list1.Count, list2.Count);
+                    for (int i = 0; i < list1.Count; i++)
+                    {
+                        Assert.Same(list1[i], list2[i]);
+                    }
+                }
+                else if (propertyType.IsValueType)
+                {
+                    if (property.Name == "ReadCommentHandling")
+                    {
+                        Assert.Equal(options.ReadCommentHandling, newOptions.ReadCommentHandling);
+                    }
+                    else
+                    {
+                        Assert.True(false, $"Public option was added to JsonSerializerOptions but not copied in the copy ctor: {property.Name}");
+                    }
+                }
+                else
+                {
+                    Assert.Same(property.GetValue(options), property.GetValue(newOptions));
+                }
+            }
+        }
     }
 }