From c4dd0cf1e1ee7bb0e938166c0e1bb99cb3917474 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 30 May 2025 18:58:57 +0300 Subject: [PATCH 1/5] Make enums serialize as strings if using the reflection-based serializer. --- .../McpJsonUtilities.cs | 6 ++++- .../McpJsonUtilitiesTests.cs | 27 ++++++++++++++++++- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpJsonUtilities.cs b/src/ModelContextProtocol.Core/McpJsonUtilities.cs index 0bc9ee777..6d8d3d2e6 100644 --- a/src/ModelContextProtocol.Core/McpJsonUtilities.cs +++ b/src/ModelContextProtocol.Core/McpJsonUtilities.cs @@ -39,8 +39,12 @@ private static JsonSerializerOptions CreateDefaultOptions() // Copy the configuration from the source generated context. JsonSerializerOptions options = new(JsonContext.Default.Options); - // Chain with all supported types from MEAI + // Chain with all supported types and converters from MEAI options.TypeInfoResolverChain.Add(AIJsonUtilities.DefaultOptions.TypeInfoResolver!); + foreach (JsonConverter converter in AIJsonUtilities.DefaultOptions.Converters) + { + options.Converters.Add(converter); + } options.MakeReadOnly(); return options; diff --git a/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs b/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs index 18dc680ef..c8a1eaeee 100644 --- a/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs +++ b/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs @@ -1,8 +1,10 @@ using System.Text.Json; +using System.Text.Json.Serialization; +using System.Text.Json.Serialization.Metadata; namespace ModelContextProtocol.Tests; -public static class McpJsonUtilitiesTests +public static partial class McpJsonUtilitiesTests { [Fact] public static void DefaultOptions_IsSingleton() @@ -22,4 +24,27 @@ public static void DefaultOptions_UseReflectionWhenEnabled() Assert.Equal(JsonSerializer.IsReflectionEnabledByDefault, options.TryGetTypeInfo(anonType, out _)); } + + [Fact] + public static void DefaultOptions_UnknownEnumHandling() + { + var options = McpJsonUtilities.DefaultOptions; + + if (JsonSerializer.IsReflectionEnabledByDefault) + { + Assert.Equal("\"A\"", JsonSerializer.Serialize(EnumWithoutAnnotation.A, options)); + Assert.Equal("\"A\"", JsonSerializer.Serialize(EnumWithAnnotation.A, options)); + } + else + { + options = new(options) { TypeInfoResolver = new DefaultJsonTypeInfoResolver() }; + Assert.Equal("1", JsonSerializer.Serialize(EnumWithoutAnnotation.A, options)); + Assert.Equal("\"A\"", JsonSerializer.Serialize(EnumWithAnnotation.A, options)); + } + } + + public enum EnumWithoutAnnotation { A = 1, B = 2, C = 3 } + + [JsonConverter(typeof(JsonStringEnumConverter))] + public enum EnumWithAnnotation { A = 1, B = 2, C = 3 } } From 4249bfa76efbb2dba0f33e03647fa41c4ecb45aa Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 30 May 2025 19:07:01 +0300 Subject: [PATCH 2/5] Remove unnecessary partial --- tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs b/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs index c8a1eaeee..e0af61eed 100644 --- a/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs +++ b/tests/ModelContextProtocol.Tests/McpJsonUtilitiesTests.cs @@ -4,7 +4,7 @@ namespace ModelContextProtocol.Tests; -public static partial class McpJsonUtilitiesTests +public static class McpJsonUtilitiesTests { [Fact] public static void DefaultOptions_IsSingleton() From bef80b6b794ede656fac618db45c8209f02d4158 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 30 May 2025 20:25:29 +0300 Subject: [PATCH 3/5] Use an explicit reflection-based converter for enums. --- .../McpJsonUtilities.cs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/src/ModelContextProtocol.Core/McpJsonUtilities.cs b/src/ModelContextProtocol.Core/McpJsonUtilities.cs index 6d8d3d2e6..67a2749c6 100644 --- a/src/ModelContextProtocol.Core/McpJsonUtilities.cs +++ b/src/ModelContextProtocol.Core/McpJsonUtilities.cs @@ -2,6 +2,7 @@ using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using System.Diagnostics.CodeAnalysis; +using System.Reflection; using System.Text.Json; using System.Text.Json.Serialization; using System.Text.Json.Serialization.Metadata; @@ -34,6 +35,7 @@ public static partial class McpJsonUtilities /// Creates default options to use for MCP-related serialization. /// /// The configured options. + [UnconditionalSuppressMessage("ReflectionAnalysis", "IL3050:RequiresDynamicCode", Justification = "Converter is guarded by IsReflectionEnabledByDefault check.")] private static JsonSerializerOptions CreateDefaultOptions() { // Copy the configuration from the source generated context. @@ -41,9 +43,9 @@ private static JsonSerializerOptions CreateDefaultOptions() // Chain with all supported types and converters from MEAI options.TypeInfoResolverChain.Add(AIJsonUtilities.DefaultOptions.TypeInfoResolver!); - foreach (JsonConverter converter in AIJsonUtilities.DefaultOptions.Converters) + if (JsonSerializer.IsReflectionEnabledByDefault) { - options.Converters.Add(converter); + options.Converters.Add(new UserDefinedJsonStringEnumConverter()); } options.MakeReadOnly(); @@ -104,6 +106,18 @@ internal static bool IsValidMcpToolSchema(JsonElement element) return AIJsonUtilities.CreateJsonSchema(returnType, serializerOptions: function.JsonSerializerOptions, inferenceOptions: schemaCreateOptions); } + + // Defines a JsonStringEnumConverter that only applies to enums outside of the current assembly. + // This is to ensure that we're not overriding the CustomizableJsonStringEnumConverter that our + // built-in enums are relying on. + [RequiresDynamicCode("Depends on JsonStringEnumConverter which requires dynamic code.")] + private sealed class UserDefinedJsonStringEnumConverter : JsonConverterFactory + { + private readonly JsonStringEnumConverter _converter = new(); + public override bool CanConvert(Type typeToConvert) => _converter.CanConvert(typeToConvert) && typeToConvert.Assembly != Assembly.GetExecutingAssembly(); + public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) => _converter.CreateConverter(typeToConvert, options); + } + // Keep in sync with CreateDefaultOptions above. [JsonSourceGenerationOptions(JsonSerializerDefaults.Web, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull, From 732d20d6d789f639891e95b8fe79e4a3ad60eec7 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Fri, 30 May 2025 20:27:34 +0300 Subject: [PATCH 4/5] Update comments --- src/ModelContextProtocol.Core/McpJsonUtilities.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/ModelContextProtocol.Core/McpJsonUtilities.cs b/src/ModelContextProtocol.Core/McpJsonUtilities.cs index 67a2749c6..05e888edd 100644 --- a/src/ModelContextProtocol.Core/McpJsonUtilities.cs +++ b/src/ModelContextProtocol.Core/McpJsonUtilities.cs @@ -41,8 +41,10 @@ private static JsonSerializerOptions CreateDefaultOptions() // Copy the configuration from the source generated context. JsonSerializerOptions options = new(JsonContext.Default.Options); - // Chain with all supported types and converters from MEAI + // Chain with all supported types from MEAI. options.TypeInfoResolverChain.Add(AIJsonUtilities.DefaultOptions.TypeInfoResolver!); + + // Add a converter for user-defined enums, if reflection is enabled by default. if (JsonSerializer.IsReflectionEnabledByDefault) { options.Converters.Add(new UserDefinedJsonStringEnumConverter()); From 7d6ee38c0f3ff2ff1983eb2cbaef4788fbf44c19 Mon Sep 17 00:00:00 2001 From: Eirik Tsarpalis Date: Thu, 12 Jun 2025 18:14:28 +0300 Subject: [PATCH 5/5] Make the reflection-based enum converter public. --- .../CustomizableJsonStringEnumConverter.cs | 27 ++++++++++++++++++- .../McpJsonUtilities.cs | 15 ++--------- 2 files changed, 28 insertions(+), 14 deletions(-) diff --git a/src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs b/src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs index 87bb430df..617b33359 100644 --- a/src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs +++ b/src/ModelContextProtocol.Core/CustomizableJsonStringEnumConverter.cs @@ -5,8 +5,8 @@ using System.Diagnostics.CodeAnalysis; #if !NET9_0_OR_GREATER using System.Reflection; -using System.Text.Json; #endif +using System.Text.Json; using System.Text.Json.Serialization; #if !NET9_0_OR_GREATER using ModelContextProtocol; @@ -66,6 +66,31 @@ public override string ConvertName(string name) => } #endif } + + /// + /// A JSON converter for enums that allows customizing the serialized string value of enum members + /// using the . + /// + /// + /// This is a temporary workaround for lack of System.Text.Json's JsonStringEnumConverter<T> + /// 9.x support for custom enum member naming. It will be replaced by the built-in functionality + /// once .NET 9 is fully adopted. + /// + [EditorBrowsable(EditorBrowsableState.Never)] + [RequiresUnreferencedCode("Requires unreferenced code to instantiate the generic enum converter.")] + [RequiresDynamicCode("Requires dynamic code to instantiate the generic enum converter.")] + public sealed class CustomizableJsonStringEnumConverter : JsonConverterFactory + { + /// + public override bool CanConvert(Type typeToConvert) => typeToConvert.IsEnum; + /// + public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) + { + Type converterType = typeof(CustomizableJsonStringEnumConverter<>).MakeGenericType(typeToConvert)!; + var factory = (JsonConverterFactory)Activator.CreateInstance(converterType)!; + return factory.CreateConverter(typeToConvert, options); + } + } } #if !NET9_0_OR_GREATER diff --git a/src/ModelContextProtocol.Core/McpJsonUtilities.cs b/src/ModelContextProtocol.Core/McpJsonUtilities.cs index 05e888edd..b075e1263 100644 --- a/src/ModelContextProtocol.Core/McpJsonUtilities.cs +++ b/src/ModelContextProtocol.Core/McpJsonUtilities.cs @@ -36,6 +36,7 @@ public static partial class McpJsonUtilities /// /// The configured options. [UnconditionalSuppressMessage("ReflectionAnalysis", "IL3050:RequiresDynamicCode", Justification = "Converter is guarded by IsReflectionEnabledByDefault check.")] + [UnconditionalSuppressMessage("Trimming", "IL2026:Members annotated with 'RequiresUnreferencedCodeAttribute' require dynamic access", Justification = "Converter is guarded by IsReflectionEnabledByDefault check.")] private static JsonSerializerOptions CreateDefaultOptions() { // Copy the configuration from the source generated context. @@ -47,7 +48,7 @@ private static JsonSerializerOptions CreateDefaultOptions() // Add a converter for user-defined enums, if reflection is enabled by default. if (JsonSerializer.IsReflectionEnabledByDefault) { - options.Converters.Add(new UserDefinedJsonStringEnumConverter()); + options.Converters.Add(new CustomizableJsonStringEnumConverter()); } options.MakeReadOnly(); @@ -108,18 +109,6 @@ internal static bool IsValidMcpToolSchema(JsonElement element) return AIJsonUtilities.CreateJsonSchema(returnType, serializerOptions: function.JsonSerializerOptions, inferenceOptions: schemaCreateOptions); } - - // Defines a JsonStringEnumConverter that only applies to enums outside of the current assembly. - // This is to ensure that we're not overriding the CustomizableJsonStringEnumConverter that our - // built-in enums are relying on. - [RequiresDynamicCode("Depends on JsonStringEnumConverter which requires dynamic code.")] - private sealed class UserDefinedJsonStringEnumConverter : JsonConverterFactory - { - private readonly JsonStringEnumConverter _converter = new(); - public override bool CanConvert(Type typeToConvert) => _converter.CanConvert(typeToConvert) && typeToConvert.Assembly != Assembly.GetExecutingAssembly(); - public override JsonConverter? CreateConverter(Type typeToConvert, JsonSerializerOptions options) => _converter.CreateConverter(typeToConvert, options); - } - // Keep in sync with CreateDefaultOptions above. [JsonSourceGenerationOptions(JsonSerializerDefaults.Web, DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,