From 2bf88ebba06f3d00703860f5d4292de67eddc228 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 30 Mar 2013 13:57:13 +1300 Subject: [PATCH] -Added support for falling back to IEnumerable constructor on lists -Added support for falling back to IEnumerable> constructor on dictionaries --- .../Serialization/JsonSerializerTest.cs | 70 +++++++- .../PreserveReferencesHandlingTests.cs | 2 +- .../Serialization/JsonArrayContract.cs | 45 ++--- .../Serialization/JsonDictionaryContract.cs | 4 + .../JsonSerializerInternalReader.cs | 162 +++++++++++------- .../Utilities/CollectionUtils.cs | 18 ++ 6 files changed, 207 insertions(+), 94 deletions(-) diff --git a/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs b/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs index 43224f74..6e8daa3a 100644 --- a/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs +++ b/Src/Newtonsoft.Json.Tests/Serialization/JsonSerializerTest.cs @@ -2723,14 +2723,23 @@ Path '', line 1, position 2.", [Test] public void DeserializeDictionaryWithNoDefaultConstructor() { - string json = "{key1:'value',key2:'value',key3:'value'}"; + string json = "{key1:'value1',key2:'value2',key3:'value3'}"; - ExceptionAssert.Throws( - "Unable to find a default constructor to use for type Newtonsoft.Json.Tests.Serialization.JsonSerializerTest+DictionaryWithNoDefaultConstructor. Path 'key1', line 1, position 6.", - () => - { - JsonConvert.DeserializeObject(json); - }); + var dic = JsonConvert.DeserializeObject(json); + + Assert.AreEqual(3, dic.Count); + Assert.AreEqual("value1", dic["key1"]); + Assert.AreEqual("value2", dic["key2"]); + Assert.AreEqual("value3", dic["key3"]); + } + + [Test] + public void DeserializeDictionaryWithNoDefaultConstructor_PreserveReferences() + { + string json = "{'$id':'1',key1:'value1',key2:'value2',key3:'value3'}"; + + ExceptionAssert.Throws("Cannot preserve reference to readonly dictionary, or dictionary created from a non-default constructor: Newtonsoft.Json.Tests.Serialization.JsonSerializerTest+DictionaryWithNoDefaultConstructor. Path 'key1', line 1, position 16.", + () => JsonConvert.DeserializeObject(json, new JsonSerializerSettings { PreserveReferencesHandling = PreserveReferencesHandling.All })); } public class DictionaryWithNoDefaultConstructor : Dictionary @@ -7941,6 +7950,53 @@ Parameter name: value", ]", json); } #endif + + public class PrivateDefaultCtorList : List + { + private PrivateDefaultCtorList() + { + } + } + + [Test] + public void DeserializePrivateListCtor() + { + ExceptionAssert.Throws( + "Unable to find a constructor to use for type Newtonsoft.Json.Tests.Serialization.JsonSerializerTest+PrivateDefaultCtorList`1[System.Int32]. Path '', line 1, position 1.", + () => JsonConvert.DeserializeObject>("[1,2]") + ); + + var list = JsonConvert.DeserializeObject>("[1,2]", new JsonSerializerSettings + { + ConstructorHandling = ConstructorHandling.AllowNonPublicDefaultConstructor + }); + + Assert.AreEqual(2, list.Count); + } + + public class PrivateDefaultCtorWithIEnumerableCtorList : List + { + private PrivateDefaultCtorWithIEnumerableCtorList() + { + } + + public PrivateDefaultCtorWithIEnumerableCtorList(IEnumerable values) + : base(values) + { + Add(default(T)); + } + } + + [Test] + public void DeserializePrivateListConstructor() + { + var list = JsonConvert.DeserializeObject>("[1,2]"); + + Assert.AreEqual(3, list.Count); + Assert.AreEqual(1, list[0]); + Assert.AreEqual(2, list[1]); + Assert.AreEqual(0, list[2]); + } } public enum Antworten diff --git a/Src/Newtonsoft.Json.Tests/Serialization/PreserveReferencesHandlingTests.cs b/Src/Newtonsoft.Json.Tests/Serialization/PreserveReferencesHandlingTests.cs index beb5ea66..48f2f862 100644 --- a/Src/Newtonsoft.Json.Tests/Serialization/PreserveReferencesHandlingTests.cs +++ b/Src/Newtonsoft.Json.Tests/Serialization/PreserveReferencesHandlingTests.cs @@ -243,7 +243,7 @@ namespace Newtonsoft.Json.Tests.Serialization }"; ExceptionAssert.Throws( - @"Cannot preserve reference to array or readonly list: System.String[][]. Path '$values', line 3, position 15.", + @"Cannot preserve reference to array or readonly list, or list created from a non-default constructor: System.String[][]. Path '$values', line 3, position 15.", () => { JsonConvert.DeserializeObject(json, diff --git a/Src/Newtonsoft.Json/Serialization/JsonArrayContract.cs b/Src/Newtonsoft.Json/Serialization/JsonArrayContract.cs index 87af9890..5961fddf 100644 --- a/Src/Newtonsoft.Json/Serialization/JsonArrayContract.cs +++ b/Src/Newtonsoft.Json/Serialization/JsonArrayContract.cs @@ -65,26 +65,8 @@ namespace Newtonsoft.Json.Serialization internal bool ShouldCreateWrapper { get; private set; } internal bool CanDeserialize { get; private set; } - internal Type TemporaryCollectionType { get; private set; } internal bool IsReadOnlyOrFixedSize { get; private set; } - - private ConstructorInfo ResolveReadOnlyCollectionConstructor(Type readOnlyCollectionType, Type collectionTypeType) - { - Type genericEnumerable = ReflectionUtils.MakeGenericType(typeof(IEnumerable<>), collectionTypeType); - - foreach (ConstructorInfo constructor in readOnlyCollectionType.GetConstructors()) - { - IList parameters = constructor.GetParameters(); - - if (parameters.Count == 1) - { - if (genericEnumerable.IsAssignableFrom(parameters[0].ParameterType)) - return constructor; - } - } - - return null; - } + internal ConstructorInfo ParametrizedConstructor { get; private set; } /// /// Initializes a new instance of the class. @@ -104,15 +86,16 @@ namespace Newtonsoft.Json.Serialization IsReadOnlyOrFixedSize = true; canDeserialize = true; - TemporaryCollectionType = typeof (List); + IsMultidimensionalArray = (UnderlyingType.IsArray && UnderlyingType.GetArrayRank() > 1); } else if (ReflectionUtils.InheritsGenericDefinition(underlyingType, typeof(ReadOnlyCollection<>), out tempCollectionType)) { CollectionItemType = tempCollectionType.GetGenericArguments()[0]; + ParametrizedConstructor = CollectionUtils.ResolveEnumableCollectionConstructor(underlyingType, CollectionItemType); + IsReadOnlyOrFixedSize = true; - canDeserialize = (ResolveReadOnlyCollectionConstructor(underlyingType, CollectionItemType) != null); - TemporaryCollectionType = ReflectionUtils.MakeGenericType(typeof(List<>), CollectionItemType); + canDeserialize = (ParametrizedConstructor != null); } else if (typeof(IList).IsAssignableFrom(underlyingType)) { @@ -121,11 +104,14 @@ namespace Newtonsoft.Json.Serialization else CollectionItemType = ReflectionUtils.GetCollectionItemType(underlyingType); + if (CollectionItemType != null) + ParametrizedConstructor = CollectionUtils.ResolveEnumableCollectionConstructor(underlyingType, CollectionItemType); canDeserialize = true; } else if (ReflectionUtils.ImplementsGenericDefinition(underlyingType, typeof(ICollection<>), out _genericCollectionDefinitionType)) { CollectionItemType = _genericCollectionDefinitionType.GetGenericArguments()[0]; + ParametrizedConstructor = CollectionUtils.ResolveEnumableCollectionConstructor(underlyingType, CollectionItemType); canDeserialize = true; ShouldCreateWrapper = true; } @@ -133,8 +119,8 @@ namespace Newtonsoft.Json.Serialization else if (ReflectionUtils.ImplementsGenericDefinition(underlyingType, typeof (IReadOnlyCollection<>), out tempCollectionType)) { CollectionItemType = underlyingType.GetGenericArguments()[0]; + ParametrizedConstructor = CollectionUtils.ResolveEnumableCollectionConstructor(underlyingType, CollectionItemType); IsReadOnlyOrFixedSize = true; - TemporaryCollectionType = ReflectionUtils.MakeGenericType(typeof(List<>), CollectionItemType); ShouldCreateWrapper = !typeof(IList).IsAssignableFrom(underlyingType); canDeserialize = (ResolveReadOnlyCollectionConstructor(underlyingType, CollectionItemType) != null @@ -145,6 +131,7 @@ namespace Newtonsoft.Json.Serialization else if (ReflectionUtils.ImplementsGenericDefinition(underlyingType, typeof (IEnumerable<>), out tempCollectionType)) { CollectionItemType = tempCollectionType.GetGenericArguments()[0]; + ParametrizedConstructor = CollectionUtils.ResolveEnumableCollectionConstructor(underlyingType, CollectionItemType); if (underlyingType.GetGenericTypeDefinition() == typeof (IEnumerable<>)) { @@ -158,8 +145,7 @@ namespace Newtonsoft.Json.Serialization { IsReadOnlyOrFixedSize = true; ShouldCreateWrapper = true; - canDeserialize = (ResolveReadOnlyCollectionConstructor(underlyingType, CollectionItemType) != null); - TemporaryCollectionType = ReflectionUtils.MakeGenericType(typeof(List<>), CollectionItemType); + canDeserialize = (ParametrizedConstructor != null); } } else @@ -195,8 +181,6 @@ namespace Newtonsoft.Json.Serialization ShouldCreateWrapper = true; } #endif - - IsMultidimensionalArray = (UnderlyingType.IsArray && UnderlyingType.GetArrayRank() > 1); } internal IWrappedCollection CreateWrapper(object list) @@ -232,7 +216,12 @@ namespace Newtonsoft.Json.Serialization internal IList CreateTemporaryCollection() { if (_genericTemporaryCollectionCreator == null) - _genericTemporaryCollectionCreator = JsonTypeReflector.ReflectionDelegateFactory.CreateDefaultConstructor(TemporaryCollectionType); + { + // multidimensional array will also have array instances in it + Type collectionItemType = (IsMultidimensionalArray) ? typeof (object) : CollectionItemType; + Type temporaryListType = ReflectionUtils.MakeGenericType(typeof(List<>), collectionItemType); + _genericTemporaryCollectionCreator = JsonTypeReflector.ReflectionDelegateFactory.CreateDefaultConstructor(temporaryListType); + } return (IList)_genericTemporaryCollectionCreator(); } diff --git a/Src/Newtonsoft.Json/Serialization/JsonDictionaryContract.cs b/Src/Newtonsoft.Json/Serialization/JsonDictionaryContract.cs index 7d529aa2..02845752 100644 --- a/Src/Newtonsoft.Json/Serialization/JsonDictionaryContract.cs +++ b/Src/Newtonsoft.Json/Serialization/JsonDictionaryContract.cs @@ -69,6 +69,7 @@ namespace Newtonsoft.Json.Serialization internal bool ShouldCreateWrapper { get; private set; } internal bool IsReadOnlyOrFixedSize { get; private set; } + internal ConstructorInfo ParametrizedConstructor { get; private set; } /// /// Initializes a new instance of the class. @@ -101,6 +102,9 @@ namespace Newtonsoft.Json.Serialization ReflectionUtils.GetDictionaryKeyValueTypes(UnderlyingType, out keyType, out valueType); } + if (keyType != null && valueType != null) + ParametrizedConstructor = CollectionUtils.ResolveEnumableCollectionConstructor(underlyingType, ReflectionUtils.MakeGenericType(typeof(KeyValuePair<,>), keyType, valueType)); + ShouldCreateWrapper = !typeof (IDictionary).IsAssignableFrom(underlyingType); DictionaryKeyType = keyType; diff --git a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs index de013ebf..d4967083 100644 --- a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs +++ b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs @@ -369,53 +369,80 @@ namespace Newtonsoft.Json.Serialization switch (contract.ContractType) { case JsonContractType.Object: - bool createdFromNonDefaultConstructor = false; - JsonObjectContract objectContract = (JsonObjectContract) contract; - object targetObject; - if (existingValue != null) - targetObject = existingValue; - else - targetObject = CreateNewObject(reader, objectContract, member, containerMember, id, out createdFromNonDefaultConstructor); + { + bool createdFromNonDefaultConstructor = false; + JsonObjectContract objectContract = (JsonObjectContract) contract; + object targetObject; + if (existingValue != null) + targetObject = existingValue; + else + targetObject = CreateNewObject(reader, objectContract, member, containerMember, id, out createdFromNonDefaultConstructor); - // don't populate if read from non-default constructor because the object has already been read - if (createdFromNonDefaultConstructor) - return targetObject; + // don't populate if read from non-default constructor because the object has already been read + if (createdFromNonDefaultConstructor) + return targetObject; - return PopulateObject(targetObject, reader, objectContract, member, id); + return PopulateObject(targetObject, reader, objectContract, member, id); + } case JsonContractType.Primitive: - JsonPrimitiveContract primitiveContract = (JsonPrimitiveContract)contract; - // if the content is inside $value then read past it - if (reader.TokenType == JsonToken.PropertyName && string.Equals(reader.Value.ToString(), JsonTypeReflector.ValuePropertyName, StringComparison.Ordinal)) { - CheckedRead(reader); + JsonPrimitiveContract primitiveContract = (JsonPrimitiveContract) contract; + // if the content is inside $value then read past it + if (reader.TokenType == JsonToken.PropertyName && string.Equals(reader.Value.ToString(), JsonTypeReflector.ValuePropertyName, StringComparison.Ordinal)) + { + CheckedRead(reader); - // the token should not be an object because the $type value could have been included in the object - // without needing the $value property - if (reader.TokenType == JsonToken.StartObject) - throw JsonSerializationException.Create(reader, "Unexpected token when deserializing primitive value: " + reader.TokenType); + // the token should not be an object because the $type value could have been included in the object + // without needing the $value property + if (reader.TokenType == JsonToken.StartObject) + throw JsonSerializationException.Create(reader, "Unexpected token when deserializing primitive value: " + reader.TokenType); - object value = CreateValueInternal(reader, objectType, primitiveContract, member, null, null, existingValue); + object value = CreateValueInternal(reader, objectType, primitiveContract, member, null, null, existingValue); - CheckedRead(reader); - return value; + CheckedRead(reader); + return value; + } + break; } - break; case JsonContractType.Dictionary: - JsonDictionaryContract dictionaryContract = (JsonDictionaryContract) contract; - object targetDictionary; - if (existingValue != null) - targetDictionary = dictionaryContract.ShouldCreateWrapper ? dictionaryContract.CreateWrapper(existingValue) : existingValue; - else - targetDictionary = CreateNewDictionary(reader, dictionaryContract); - - object dictionary = PopulateDictionary((IDictionary) targetDictionary, reader, dictionaryContract, member, id); - - if (dictionaryContract.IsReadOnlyOrFixedSize) { - dictionary = ReflectionUtils.CreateInstance(contract.CreatedType, dictionary); - } + JsonDictionaryContract dictionaryContract = (JsonDictionaryContract) contract; + object targetDictionary; - return dictionary; + if (existingValue == null) + { + bool createdFromNonDefaultConstructor; + IDictionary dictionary = CreateNewDictionary(reader, dictionaryContract, out createdFromNonDefaultConstructor); + + if (id != null && createdFromNonDefaultConstructor) + throw JsonSerializationException.Create(reader, "Cannot preserve reference to readonly dictionary, or dictionary created from a non-default constructor: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); + + if (contract.OnSerializingCallbacks.Count > 0 && createdFromNonDefaultConstructor) + throw JsonSerializationException.Create(reader, "Cannot call OnSerializing on readonly dictionary, or dictionary created from a non-default constructor: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); + + if (contract.OnErrorCallbacks.Count > 0 && createdFromNonDefaultConstructor) + throw JsonSerializationException.Create(reader, "Cannot call OnError on readonly list, or dictionary created from a non-default constructor: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); + + PopulateDictionary(dictionary, reader, dictionaryContract, member, id); + + if (createdFromNonDefaultConstructor) + { + return dictionaryContract.ParametrizedConstructor.Invoke(new object[] {dictionary}); + } + else if (dictionary is IWrappedDictionary) + { + return ((IWrappedDictionary)dictionary).UnderlyingDictionary; + } + + targetDictionary = dictionary; + } + else + { + targetDictionary = PopulateDictionary(dictionaryContract.ShouldCreateWrapper ? dictionaryContract.CreateWrapper(existingValue) : (IDictionary) existingValue, reader, dictionaryContract, member, id); + } + + return targetDictionary; + } #if !(NET35 || NET20) case JsonContractType.Dynamic: JsonDynamicContract dynamicContract = (JsonDynamicContract) contract; @@ -584,25 +611,24 @@ To fix this error either change the JSON to a {1} or change the deserialized typ if (existingValue == null) { - if (id != null && arrayContract.IsReadOnlyOrFixedSize) - throw JsonSerializationException.Create(reader, "Cannot preserve reference to array or readonly list: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); + bool createdFromNonDefaultConstructor; + IList list = CreateNewList(reader, arrayContract, out createdFromNonDefaultConstructor); - if (contract.OnSerializingCallbacks.Count > 0 && arrayContract.IsReadOnlyOrFixedSize) - throw JsonSerializationException.Create(reader, "Cannot call OnSerializing on an array or readonly list: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); + if (id != null && createdFromNonDefaultConstructor) + throw JsonSerializationException.Create(reader, "Cannot preserve reference to array or readonly list, or list created from a non-default constructor: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); - if (contract.OnErrorCallbacks.Count > 0 && arrayContract.IsReadOnlyOrFixedSize) - throw JsonSerializationException.Create(reader, "Cannot call OnError on an array or readonly list: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); + if (contract.OnSerializingCallbacks.Count > 0 && createdFromNonDefaultConstructor) + throw JsonSerializationException.Create(reader, "Cannot call OnSerializing on an array or readonly list, or list created from a non-default constructor: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); - IList list = CreateNewList(reader, arrayContract); + if (contract.OnErrorCallbacks.Count > 0 && createdFromNonDefaultConstructor) + throw JsonSerializationException.Create(reader, "Cannot call OnError on an array or readonly list, or list created from a non-default constructor: {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); if (!arrayContract.IsMultidimensionalArray) PopulateList(list, reader, arrayContract, member, id); else PopulateMultidimensionalArray(list, reader, arrayContract, member, id); - // create readonly and fixed sized collections using the temporary list - // not needed for IEnumerable properties. temp list is final - if (arrayContract.IsReadOnlyOrFixedSize && list.GetType() != contract.CreatedType) + if (createdFromNonDefaultConstructor) { if (arrayContract.IsMultidimensionalArray) { @@ -610,11 +636,14 @@ To fix this error either change the JSON to a {1} or change the deserialized typ } else if (contract.CreatedType.IsArray) { - list = CollectionUtils.ToArray(((List)list).ToArray(), arrayContract.CollectionItemType); + Array a = Array.CreateInstance(arrayContract.CollectionItemType, list.Count); + list.CopyTo(a, 0); + list = a; } else { - return ReflectionUtils.CreateInstance(contract.CreatedType, list); + // call constructor that takes IEnumerable + return arrayContract.ParametrizedConstructor.Invoke(new object[] { list }); } } else if (list is IWrappedCollection) @@ -837,47 +866,64 @@ To fix this error either change the JSON to a {1} or change the deserialized typ return true; } - private IList CreateNewList(JsonReader reader, JsonArrayContract contract) + private IList CreateNewList(JsonReader reader, JsonArrayContract contract, out bool createdFromNonDefaultConstructor) { + // some types like non-generic IEnumerable can be serialized but not deserialized if (!contract.CanDeserialize) throw JsonSerializationException.Create(reader, "Cannot create and populate list type {0}.".FormatWith(CultureInfo.InvariantCulture, contract.CreatedType)); - if (contract.IsReadOnlyOrFixedSize && contract.TemporaryCollectionType != null) + if (contract.IsReadOnlyOrFixedSize) { + createdFromNonDefaultConstructor = true; return contract.CreateTemporaryCollection(); } - else + else if (contract.DefaultCreator != null && (!contract.DefaultCreatorNonPublic || Serializer.ConstructorHandling == ConstructorHandling.AllowNonPublicDefaultConstructor)) { object list = contract.DefaultCreator(); if (contract.ShouldCreateWrapper) list = contract.CreateWrapper(list); + createdFromNonDefaultConstructor = false; return (IList)list; } + else if (contract.ParametrizedConstructor != null) + { + createdFromNonDefaultConstructor = true; + return contract.CreateTemporaryCollection(); + } + else + { + throw JsonSerializationException.Create(reader, "Unable to find a constructor to use for type {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); + } } - private IDictionary CreateNewDictionary(JsonReader reader, JsonDictionaryContract contract) + private IDictionary CreateNewDictionary(JsonReader reader, JsonDictionaryContract contract, out bool createdFromNonDefaultConstructor) { - object dictionary; - if (contract.IsReadOnlyOrFixedSize) { - dictionary = contract.CreateTemporaryDictionary(); + createdFromNonDefaultConstructor = true; + return contract.CreateTemporaryDictionary(); } else if (contract.DefaultCreator != null && (!contract.DefaultCreatorNonPublic || Serializer.ConstructorHandling == ConstructorHandling.AllowNonPublicDefaultConstructor)) { - dictionary = contract.DefaultCreator(); + object dictionary = contract.DefaultCreator(); if (contract.ShouldCreateWrapper) dictionary = contract.CreateWrapper(dictionary); + + createdFromNonDefaultConstructor = false; + return (IDictionary) dictionary; + } + else if (contract.ParametrizedConstructor != null) + { + createdFromNonDefaultConstructor = true; + return contract.CreateTemporaryDictionary(); } else { throw JsonSerializationException.Create(reader, "Unable to find a default constructor to use for type {0}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType)); } - - return (IDictionary)dictionary; } private void OnDeserializing(JsonReader reader, JsonContract contract, object value) diff --git a/Src/Newtonsoft.Json/Utilities/CollectionUtils.cs b/Src/Newtonsoft.Json/Utilities/CollectionUtils.cs index 8f294191..0ca49188 100644 --- a/Src/Newtonsoft.Json/Utilities/CollectionUtils.cs +++ b/Src/Newtonsoft.Json/Utilities/CollectionUtils.cs @@ -114,6 +114,24 @@ namespace Newtonsoft.Json.Utilities return false; } + public static ConstructorInfo ResolveEnumableCollectionConstructor(Type collectionType, Type collectionItemType) + { + Type genericEnumerable = ReflectionUtils.MakeGenericType(typeof(IEnumerable<>), collectionItemType); + + foreach (ConstructorInfo constructor in collectionType.GetConstructors(BindingFlags.Public | BindingFlags.Instance)) + { + IList parameters = constructor.GetParameters(); + + if (parameters.Count == 1) + { + if (genericEnumerable.IsAssignableFrom(parameters[0].ParameterType)) + return constructor; + } + } + + return null; + } + public static IWrappedCollection CreateCollectionWrapper(object list) { ValidationUtils.ArgumentNotNull(list, "list");