From a64616f9f9634e180f2803a45d9d7a7519aed5f1 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sun, 6 Dec 2015 20:26:55 +1300 Subject: [PATCH] -Fixed setting default values onto properties already set in constructor when deserializing --- .../DefaultValueHandlingTests.cs | 80 +++++ .../JsonSerializerInternalReader.cs | 304 ++++++++++-------- 2 files changed, 248 insertions(+), 136 deletions(-) diff --git a/Src/Newtonsoft.Json.Tests/Serialization/DefaultValueHandlingTests.cs b/Src/Newtonsoft.Json.Tests/Serialization/DefaultValueHandlingTests.cs index eb5d02cc..8f4a0a62 100644 --- a/Src/Newtonsoft.Json.Tests/Serialization/DefaultValueHandlingTests.cs +++ b/Src/Newtonsoft.Json.Tests/Serialization/DefaultValueHandlingTests.cs @@ -383,6 +383,79 @@ namespace Newtonsoft.Json.Tests.Serialization [DefaultValue(6)] public int IntValue { get; set; } } + + public sealed class FieldExportFormat + { + private string _format; + private ExportFormat? _exportFormat; + + [JsonProperty] + public ExportFormat? ExportFormat + { + get { return _exportFormat; } + private set + { + if (!value.HasValue) + { + throw new ArgumentNullException("ExportFormat"); + } + _exportFormat = value; + _format = null; + } + } + + [JsonProperty] + public string Format + { + get { return _format; } + private set + { + if (value == null) + { + throw new ArgumentNullException("Format"); + } + _format = value; + _exportFormat = null; + } + } + + public FieldExportFormat(string format) + { + Format = format; + } + + public FieldExportFormat(ExportFormat exportFormat) + { + ExportFormat = exportFormat; + } + + [JsonConstructor] + private FieldExportFormat(string format, ExportFormat? exportFormat) + { + if (exportFormat.HasValue) + { + ExportFormat = exportFormat; + } + else + { + Format = format; + } + } + } + + [Test] + public void DontSetPropertiesDefaultValueUsedInConstructor() + { + string json = @"{""ExportFormat"":0}"; + + FieldExportFormat o = JsonConvert.DeserializeObject(json, new JsonSerializerSettings + { + DefaultValueHandling = DefaultValueHandling.Populate + }); + + Assert.AreEqual(ExportFormat.Default, o.ExportFormat); + Assert.AreEqual(null, o.Format); + } } #if !NET20 @@ -529,4 +602,11 @@ namespace Newtonsoft.Json.Tests.Serialization public object Object { get; set; } } #endif + + public enum ExportFormat + { + Default = 0, + Currency, + Integer + } } \ No newline at end of file diff --git a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs index 3d4b32a3..4c78b805 100644 --- a/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs +++ b/Src/Newtonsoft.Json/Serialization/JsonSerializerInternalReader.cs @@ -1890,14 +1890,22 @@ namespace Newtonsoft.Json.Serialization } #endif + internal class CreatorPropertyContext + { + public string Name; + public JsonProperty Property; + public JsonProperty ConstructorProperty; + public PropertyPresence? Presence; + public object Value; + public bool Used; + } + private object CreateObjectUsingCreatorWithParameters(JsonReader reader, JsonObjectContract contract, JsonProperty containerProperty, ObjectConstructor creator, string id) { ValidationUtils.ArgumentNotNull(creator, "creator"); // only need to keep a track of properies presence if they are required or a value should be defaulted if missing - Dictionary propertiesPresence = (contract.HasRequiredOrDefaultValueProperties || HasFlag(Serializer._defaultValueHandling, DefaultValueHandling.Populate)) - ? contract.Properties.ToDictionary(m => m, m => PropertyPresence.None) - : null; + bool trackPresence = (contract.HasRequiredOrDefaultValueProperties || HasFlag(Serializer._defaultValueHandling, DefaultValueHandling.Populate)); Type objectType = contract.UnderlyingType; @@ -1907,44 +1915,33 @@ namespace Newtonsoft.Json.Serialization TraceWriter.Trace(TraceLevel.Info, JsonPosition.FormatMessage(reader as IJsonLineInfo, reader.Path, "Deserializing {0} using creator with parameters: {1}.".FormatWith(CultureInfo.InvariantCulture, contract.UnderlyingType, parameters)), null); } - IDictionary extensionData; - IDictionary propertyValues = ResolvePropertyAndCreatorValues(contract, containerProperty, reader, objectType, out extensionData); + List propertyContexts = ResolvePropertyAndCreatorValues(contract, containerProperty, reader, objectType); + if (trackPresence) + { + foreach (JsonProperty property in contract.Properties) + { + if (propertyContexts.All(p => p.Property != property)) + { + propertyContexts.Add(new CreatorPropertyContext + { + Property = property, + Name = property.PropertyName, + Presence = PropertyPresence.None + }); + } + } + } object[] creatorParameterValues = new object[contract.CreatorParameters.Count]; - IDictionary remainingPropertyValues = new Dictionary(); - foreach (KeyValuePair propertyValue in propertyValues) + foreach (CreatorPropertyContext context in propertyContexts) { - JsonProperty property = propertyValue.Key; - - JsonProperty matchingCreatorParameter; - if (contract.CreatorParameters.Contains(property)) + // set presence of read values + if (trackPresence) { - matchingCreatorParameter = property; - } - else - { - // check to see if a parameter with the same name as the underlying property name exists and match to that - matchingCreatorParameter = contract.CreatorParameters.ForgivingCaseSensitiveFind(p => p.PropertyName, property.UnderlyingName); - } - - if (matchingCreatorParameter != null) - { - int i = contract.CreatorParameters.IndexOf(matchingCreatorParameter); - creatorParameterValues[i] = propertyValue.Value; - } - else - { - remainingPropertyValues.Add(propertyValue); - } - - if (propertiesPresence != null) - { - // map from creator property to normal property - JsonProperty presenceProperty = propertiesPresence.Keys.ForgivingCaseSensitiveFind(p => p.PropertyName, property.PropertyName); - if (presenceProperty != null) + if (context.Property != null && context.Presence == null) { - object v = propertyValue.Value; + object v = context.Value; PropertyPresence propertyPresence; if (v == null) { @@ -1952,7 +1949,7 @@ namespace Newtonsoft.Json.Serialization } else if (v is string) { - propertyPresence = CoerceEmptyStringToNull(property.PropertyType, property.PropertyContract, (string)v) + propertyPresence = CoerceEmptyStringToNull(context.Property.PropertyType, context.Property.PropertyContract, (string)v) ? PropertyPresence.Null : PropertyPresence.Value; } @@ -1961,38 +1958,45 @@ namespace Newtonsoft.Json.Serialization propertyPresence = PropertyPresence.Value; } - propertiesPresence[presenceProperty] = propertyPresence; + context.Presence = propertyPresence; } } - } - // handle giving default values to creator parameters - // this needs to happen before the call to creator - if (propertiesPresence != null) - { - foreach (KeyValuePair propertyPresence in propertiesPresence) + JsonProperty constructorProperty = context.ConstructorProperty; + if (constructorProperty == null && context.Property != null) { - JsonProperty property = propertyPresence.Key; - PropertyPresence presence = propertyPresence.Value; + constructorProperty = contract.CreatorParameters.ForgivingCaseSensitiveFind(p => p.PropertyName, context.Property.UnderlyingName); + } - if (!property.Ignored && (presence == PropertyPresence.None || presence == PropertyPresence.Null)) + if (constructorProperty != null && !constructorProperty.Ignored) + { + // handle giving default values to creator parameters + // this needs to happen before the call to creator + if (trackPresence) { - if (property.PropertyContract == null) + if (context.Presence == PropertyPresence.None || context.Presence == PropertyPresence.Null) { - property.PropertyContract = GetContractSafe(property.PropertyType); - } - - if (HasFlag(property.DefaultValueHandling.GetValueOrDefault(Serializer._defaultValueHandling), DefaultValueHandling.Populate)) - { - // have to use the name because property presence doesn't use creator properties - int i = contract.CreatorParameters.IndexOf(p => p.PropertyName == property.PropertyName); - - if (i != -1) + if (constructorProperty.PropertyContract == null) { - creatorParameterValues[i] = EnsureType(reader, property.GetResolvedDefaultValue(), CultureInfo.InvariantCulture, property.PropertyContract, property.PropertyType); + constructorProperty.PropertyContract = GetContractSafe(constructorProperty.PropertyType); + } + + if (HasFlag(constructorProperty.DefaultValueHandling.GetValueOrDefault(Serializer._defaultValueHandling), DefaultValueHandling.Populate)) + { + context.Value = EnsureType( + reader, + constructorProperty.GetResolvedDefaultValue(), + CultureInfo.InvariantCulture, + constructorProperty.PropertyContract, + constructorProperty.PropertyType); } } } + + int i = contract.CreatorParameters.IndexOf(constructorProperty); + creatorParameterValues[i] = context.Value; + + context.Used = true; } } @@ -2006,14 +2010,23 @@ namespace Newtonsoft.Json.Serialization OnDeserializing(reader, contract, createdObject); // go through unused values and set the newly created object's properties - foreach (KeyValuePair remainingPropertyValue in remainingPropertyValues) + foreach (CreatorPropertyContext context in propertyContexts) { - JsonProperty property = remainingPropertyValue.Key; - object value = remainingPropertyValue.Value; + if (context.Used || + context.Property == null || + context.Property.Ignored || + context.Presence == PropertyPresence.None) + { + continue; + } + + JsonProperty property = context.Property; + object value = context.Value; if (ShouldSetPropertyValue(property, value)) { property.ValueProvider.SetValue(createdObject, value); + context.Used = true; } else if (!property.Writable && value != null) { @@ -2052,18 +2065,41 @@ namespace Newtonsoft.Json.Serialization } } } + + context.Used = true; } } - if (extensionData != null) + if (contract.ExtensionDataSetter != null) { - foreach (KeyValuePair e in extensionData) + foreach (CreatorPropertyContext propertyValue in propertyContexts) { - contract.ExtensionDataSetter(createdObject, e.Key, e.Value); + if (!propertyValue.Used) + { + contract.ExtensionDataSetter(createdObject, propertyValue.Name, propertyValue.Value); + } } } - EndObject(createdObject, reader, contract, reader.Depth, propertiesPresence); + if (trackPresence) + { + foreach (CreatorPropertyContext context in propertyContexts) + { + if (context.Property == null) + { + continue; + } + + EndProcessProperty( + createdObject, + reader, + contract, + reader.Depth, + context.Property, + context.Presence.Value, + !context.Used); + } + } OnDeserialized(reader, contract, createdObject); return createdObject; @@ -2086,11 +2122,9 @@ namespace Newtonsoft.Json.Serialization return value; } - private IDictionary ResolvePropertyAndCreatorValues(JsonObjectContract contract, JsonProperty containerProperty, JsonReader reader, Type objectType, out IDictionary extensionData) + private List ResolvePropertyAndCreatorValues(JsonObjectContract contract, JsonProperty containerProperty, JsonReader reader, Type objectType) { - extensionData = (contract.ExtensionDataSetter != null) ? new Dictionary() : null; - - IDictionary propertyValues = new Dictionary(); + List propertyValues = new List(); bool exit = false; do { @@ -2099,12 +2133,16 @@ namespace Newtonsoft.Json.Serialization case JsonToken.PropertyName: string memberName = reader.Value.ToString(); - // attempt exact case match first - // then try match ignoring case - JsonProperty property = contract.CreatorParameters.GetClosestMatchProperty(memberName) ?? - contract.Properties.GetClosestMatchProperty(memberName); + CreatorPropertyContext creatorPropertyContext = new CreatorPropertyContext + { + Name = reader.Value.ToString(), + ConstructorProperty = contract.CreatorParameters.GetClosestMatchProperty(memberName), + Property = contract.Properties.GetClosestMatchProperty(memberName) + }; + propertyValues.Add(creatorPropertyContext); - if (property != null) + JsonProperty property = creatorPropertyContext.ConstructorProperty ?? creatorPropertyContext.Property; + if (property != null && !property.Ignored) { if (property.PropertyContract == null) { @@ -2118,21 +2156,16 @@ namespace Newtonsoft.Json.Serialization throw JsonSerializationException.Create(reader, "Unexpected end when setting {0}'s value.".FormatWith(CultureInfo.InvariantCulture, memberName)); } - if (!property.Ignored) + if (propertyConverter != null && propertyConverter.CanRead) { - object propertyValue; - if (propertyConverter != null && propertyConverter.CanRead) - { - propertyValue = DeserializeConvertable(propertyConverter, reader, property.PropertyType, null); - } - else - { - propertyValue = CreateValueInternal(reader, property.PropertyType, property.PropertyContract, property, contract, containerProperty, null); - } - - propertyValues[property] = propertyValue; - continue; + creatorPropertyContext.Value = DeserializeConvertable(propertyConverter, reader, property.PropertyType, null); } + else + { + creatorPropertyContext.Value = CreateValueInternal(reader, property.PropertyType, property.PropertyContract, property, contract, containerProperty, null); + } + + continue; } else { @@ -2152,10 +2185,9 @@ namespace Newtonsoft.Json.Serialization } } - if (extensionData != null) + if (contract.ExtensionDataSetter != null) { - object value = ReadExtensionDataValue(contract, containerProperty, reader); - extensionData[memberName] = value; + creatorPropertyContext.Value = ReadExtensionDataValue(contract, containerProperty, reader); } else { @@ -2387,7 +2419,16 @@ namespace Newtonsoft.Json.Serialization ThrowUnexpectedEndException(reader, contract, newObject, "Unexpected end when deserializing object."); } - EndObject(newObject, reader, contract, initialDepth, propertiesPresence); + if (propertiesPresence != null) + { + foreach (KeyValuePair propertyPresence in propertiesPresence) + { + JsonProperty property = propertyPresence.Key; + PropertyPresence presence = propertyPresence.Value; + + EndProcessProperty(newObject, reader, contract, initialDepth, property, presence, true); + } + } OnDeserialized(reader, contract, newObject); return newObject; @@ -2462,65 +2503,56 @@ namespace Newtonsoft.Json.Serialization return value; } - private void EndObject(object newObject, JsonReader reader, JsonObjectContract contract, int initialDepth, Dictionary propertiesPresence) + private void EndProcessProperty(object newObject, JsonReader reader, JsonObjectContract contract, int initialDepth, JsonProperty property, PropertyPresence presence, bool setDefaultValue) { - if (propertiesPresence != null) + if (presence == PropertyPresence.None || presence == PropertyPresence.Null) { - foreach (KeyValuePair propertyPresence in propertiesPresence) + try { - JsonProperty property = propertyPresence.Key; - PropertyPresence presence = propertyPresence.Value; + Required resolvedRequired = property._required ?? contract.ItemRequired ?? Required.Default; - if (presence == PropertyPresence.None || presence == PropertyPresence.Null) + switch (presence) { - try - { - Required resolvedRequired = property._required ?? contract.ItemRequired ?? Required.Default; - - switch (presence) + case PropertyPresence.None: + if (resolvedRequired == Required.AllowNull || resolvedRequired == Required.Always) { - case PropertyPresence.None: - if (resolvedRequired == Required.AllowNull || resolvedRequired == Required.Always) - { - throw JsonSerializationException.Create(reader, "Required property '{0}' not found in JSON.".FormatWith(CultureInfo.InvariantCulture, property.PropertyName)); - } + throw JsonSerializationException.Create(reader, "Required property '{0}' not found in JSON.".FormatWith(CultureInfo.InvariantCulture, property.PropertyName)); + } - if (!property.Ignored) - { - if (property.PropertyContract == null) - { - property.PropertyContract = GetContractSafe(property.PropertyType); - } + if (setDefaultValue && !property.Ignored) + { + if (property.PropertyContract == null) + { + property.PropertyContract = GetContractSafe(property.PropertyType); + } - if (HasFlag(property.DefaultValueHandling.GetValueOrDefault(Serializer._defaultValueHandling), DefaultValueHandling.Populate) && property.Writable) - { - property.ValueProvider.SetValue(newObject, EnsureType(reader, property.GetResolvedDefaultValue(), CultureInfo.InvariantCulture, property.PropertyContract, property.PropertyType)); - } - } - break; - case PropertyPresence.Null: - if (resolvedRequired == Required.Always) - { - throw JsonSerializationException.Create(reader, "Required property '{0}' expects a value but got null.".FormatWith(CultureInfo.InvariantCulture, property.PropertyName)); - } - if (resolvedRequired == Required.DisallowNull) - { - throw JsonSerializationException.Create(reader, "Required property '{0}' expects a non-null value.".FormatWith(CultureInfo.InvariantCulture, property.PropertyName)); - } - break; + if (HasFlag(property.DefaultValueHandling.GetValueOrDefault(Serializer._defaultValueHandling), DefaultValueHandling.Populate) && property.Writable) + { + property.ValueProvider.SetValue(newObject, EnsureType(reader, property.GetResolvedDefaultValue(), CultureInfo.InvariantCulture, property.PropertyContract, property.PropertyType)); + } } - } - catch (Exception ex) - { - if (IsErrorHandled(newObject, contract, property.PropertyName, reader as IJsonLineInfo, reader.Path, ex)) + break; + case PropertyPresence.Null: + if (resolvedRequired == Required.Always) { - HandleError(reader, true, initialDepth); + throw JsonSerializationException.Create(reader, "Required property '{0}' expects a value but got null.".FormatWith(CultureInfo.InvariantCulture, property.PropertyName)); } - else + if (resolvedRequired == Required.DisallowNull) { - throw; + throw JsonSerializationException.Create(reader, "Required property '{0}' expects a non-null value.".FormatWith(CultureInfo.InvariantCulture, property.PropertyName)); } - } + break; + } + } + catch (Exception ex) + { + if (IsErrorHandled(newObject, contract, property.PropertyName, reader as IJsonLineInfo, reader.Path, ex)) + { + HandleError(reader, true, initialDepth); + } + else + { + throw; } } }