From c4bc6e8f28509d7c8ae69914283b67c681c87fbd Mon Sep 17 00:00:00 2001 From: Layomi Akinrinade Date: Wed, 9 Jun 2021 13:57:39 -0700 Subject: [PATCH] Fix serializer check to permit fast-path serialization logic to run (#53916) * Fix serializer check to permit fast-path serialization logic to run * Address feedback * Add more tests * Address feedback --- .../Json/Serialization/JsonSerializerContext.cs | 8 ++- .../SerializationLogicTests.cs | 70 +++++++++++++++++++--- 2 files changed, 69 insertions(+), 9 deletions(-) diff --git a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs index 78843cf..77f723b 100644 --- a/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs +++ b/src/libraries/System.Text.Json/src/System/Text/Json/Serialization/JsonSerializerContext.cs @@ -37,8 +37,14 @@ namespace System.Text.Json.Serialization // Guard against unsupported features Options.Converters.Count == 0 && Options.Encoder == null && - Options.NumberHandling == JsonNumberHandling.Strict && + // Disallow custom number handling we'd need to honor when writing. + // AllowReadingFromString and Strict are fine since there's no action to take when writing. + (Options.NumberHandling & (JsonNumberHandling.WriteAsString | JsonNumberHandling.AllowNamedFloatingPointLiterals)) == 0 && Options.ReferenceHandlingStrategy == ReferenceHandlingStrategy.None && +#pragma warning disable SYSLIB0020 + !Options.IgnoreNullValues && // This property is obsolete. +#pragma warning restore SYSLIB0020 + // Ensure options values are consistent with expected defaults. Options.DefaultIgnoreCondition == _defaultOptions.DefaultIgnoreCondition && Options.IgnoreReadOnlyFields == _defaultOptions.IgnoreReadOnlyFields && diff --git a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationLogicTests.cs b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationLogicTests.cs index 87d8822..53474be 100644 --- a/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationLogicTests.cs +++ b/src/libraries/System.Text.Json/tests/System.Text.Json.SourceGeneration.Tests/SerializationLogicTests.cs @@ -10,6 +10,25 @@ namespace System.Text.Json.SourceGeneration.Tests { public static class SerializationLogicTests { + private static JsonSerializerOptions s_compatibleOptions = new JsonSerializerOptions + { + DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase + }; + + [Theory] + [MemberData(nameof(GetOptionsUsingDeserializationOnlyFeatures))] + [MemberData(nameof(GetCompatibleOptions))] + public static void SerializationFuncInvokedWhenSupported(JsonSerializerOptions options) + { + JsonMessage message = new(); + + // Per context implementation, NotImplementedException thrown because the options are compatible, hence the serialization func is invoked. + JsonContext context = new(options); + Assert.Throws(() => JsonSerializer.Serialize(message, context.JsonMessage)); + Assert.Throws(() => JsonSerializer.Serialize(message, typeof(JsonMessage), context)); + } + [Theory] [MemberData(nameof(GetOptionsUsingUnsupportedFeatures))] [MemberData(nameof(GetIncompatibleOptions))] @@ -39,23 +58,58 @@ namespace System.Text.Json.SourceGeneration.Tests Assert.Null(DictionaryTypeContext.Default.Int32.Serialize); } - // Options with features that aren't supported in generated serialization funcs. + // Options with features that apply only to deserialization. + public static IEnumerable GetOptionsUsingDeserializationOnlyFeatures() + { + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { AllowTrailingCommas = true } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { DefaultBufferSize = 8192 } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { PropertyNameCaseInsensitive = true } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { ReadCommentHandling = JsonCommentHandling.Skip } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { NumberHandling = JsonNumberHandling.AllowReadingFromString } }; + } + + /// + /// Options compatible with . + /// + public static IEnumerable GetCompatibleOptions() + { + yield return new object[] { s_compatibleOptions }; + yield return new object[] { new JsonSerializerOptions(JsonSerializerDefaults.Web) { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { NumberHandling = JsonNumberHandling.Strict } }; + } + + // Options with features that aren't supported in the generated serialization funcs. public static IEnumerable GetOptionsUsingUnsupportedFeatures() { - yield return new object[] { new JsonSerializerOptions { Converters = { new JsonStringEnumConverter() } } }; - yield return new object[] { new JsonSerializerOptions { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping } }; - yield return new object[] { new JsonSerializerOptions { NumberHandling = JsonNumberHandling.WriteAsString } }; - yield return new object[] { new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.Preserve } }; - yield return new object[] { new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.IgnoreCycles } }; - yield return new object[] { new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.IgnoreCycles } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { Converters = { new JsonStringEnumConverter() } } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { NumberHandling = JsonNumberHandling.WriteAsString } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { NumberHandling = JsonNumberHandling.AllowNamedFloatingPointLiterals } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { NumberHandling = JsonNumberHandling.WriteAsString | JsonNumberHandling.AllowNamedFloatingPointLiterals } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { ReferenceHandler = ReferenceHandler.Preserve } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { ReferenceHandler = ReferenceHandler.IgnoreCycles } }; +#pragma warning disable SYSLIB0020 // Type or member is obsolete + yield return new object[] { new JsonSerializerOptions { IgnoreNullValues = true, PropertyNamingPolicy = JsonNamingPolicy.CamelCase } }; +#pragma warning restore SYSLIB0020 // Type or member is obsolete } - // Options incompatible with JsonContext.s_defaultOptions below. + /// + /// Options incompatible with . + /// public static IEnumerable GetIncompatibleOptions() { yield return new object[] { new JsonSerializerOptions() }; + yield return new object[] { new JsonSerializerOptions { Converters = { new JsonStringEnumConverter() } } }; + yield return new object[] { new JsonSerializerOptions { Encoder = JavaScriptEncoder.UnsafeRelaxedJsonEscaping } }; + yield return new object[] { new JsonSerializerOptions { NumberHandling = JsonNumberHandling.WriteAsString } }; + yield return new object[] { new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.Preserve } }; + yield return new object[] { new JsonSerializerOptions { ReferenceHandler = ReferenceHandler.IgnoreCycles } }; yield return new object[] { new JsonSerializerOptions { DefaultIgnoreCondition = JsonIgnoreCondition.Never } }; yield return new object[] { new JsonSerializerOptions { IgnoreReadOnlyFields = true } }; + yield return new object[] { new JsonSerializerOptions { DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingDefault } }; + yield return new object[] { new JsonSerializerOptions { PropertyNamingPolicy = JsonNamingPolicy.CamelCase } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { DefaultIgnoreCondition = JsonIgnoreCondition.Never } }; + yield return new object[] { new JsonSerializerOptions(s_compatibleOptions) { IgnoreReadOnlyFields = true } }; } } } -- 2.7.4