Avoid new JsonSerializerOptions instances (#34)

We try hard to avoid creating unnecessary JsonSerializerOptions, as each can have a non-trivial overhead due to how reflection is used by JsonSerializer and the JSO instance serving as a cache.  Rather than creating a new JSO for each transport / endpoint, this can just use a singleton. The only thing it loses is logging when the converter fails, but all of those paths are also throwing an exception, which can be logged higher in the call stack.
This commit is contained in:
Stephen Toub 2025-03-07 13:50:27 -05:00 • committed by GitHub
commit 9f0beec63a
No known key found for this signature in database
GPG key ID: B5690EEEBB952194

View file

@ -9,21 +9,13 @@ namespace McpDotNet.Utils.Json;
/// <summary>
/// JSON converter for IJsonRpcMessage that handles polymorphic deserialization of different message types.
/// </summary>
internal class JsonRpcMessageConverter : JsonConverter<IJsonRpcMessage>
internal sealed class JsonRpcMessageConverter : JsonConverter<IJsonRpcMessage>
{
private readonly ILogger<JsonRpcMessageConverter> _logger;
public JsonRpcMessageConverter(ILogger<JsonRpcMessageConverter> logger)
{
_logger = logger;
}
/// <inheritdoc/>
public override IJsonRpcMessage? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.StartObject)
{
_logger.JsonRpcMessageConverterExpectedStartObjectToken();
throw new JsonException("Expected StartObject token");
}
@ -34,7 +26,6 @@ internal class JsonRpcMessageConverter : JsonConverter<IJsonRpcMessage>
if (!root.TryGetProperty("jsonrpc", out var versionProperty) ||
versionProperty.GetString() != "2.0")
{
_logger.JsonRpcMessageConverterInvalidJsonRpcVersion();
throw new JsonException("Invalid or missing jsonrpc version");
}
@ -51,32 +42,27 @@ internal class JsonRpcMessageConverter : JsonConverter<IJsonRpcMessage>
// Messages with an error property are error responses
if (hasError)
{
_logger.JsonRpcMessageConverterDeserializingErrorResponse(rawText);
return JsonSerializer.Deserialize<JsonRpcError>(rawText, options);
}
// Messages with a result property are success responses
else if (root.TryGetProperty("result", out _))
{
_logger.JsonRpcMessageConverterDeserializingResponse(rawText);
return JsonSerializer.Deserialize<JsonRpcResponse>(rawText, options);
}
_logger.JsonRpcMessageConverterResponseMustHaveResultOrError(rawText);
throw new JsonException("Response must have either result or error");
}
// Messages with a method but no id are notifications
else if (hasMethod && !hasId)
{
_logger.JsonRpcMessageConverterDeserializingNotification(rawText);
return JsonSerializer.Deserialize<JsonRpcNotification>(rawText, options);
}
// Messages with both method and id are requests
else if (hasMethod && hasId)
{
_logger.JsonRpcMessageConverterDeserializingRequest(rawText);
return JsonSerializer.Deserialize<JsonRpcRequest>(rawText, options);
}
_logger.JsonRpcMessageConverterInvalidMessageFormat(rawText);
throw new JsonException("Invalid JSON-RPC message format");
}
@ -98,7 +84,6 @@ internal class JsonRpcMessageConverter : JsonConverter<IJsonRpcMessage>
JsonSerializer.Serialize(writer, error, options);
break;
default:
_logger.JsonRpcMessageConverterWriteUnknownMessageType(value.GetType().ToString());
throw new JsonException($"Unknown JSON-RPC message type: {value.GetType()}");
}
}