Add MessagePackFormatter.InternStrings property (#1446)

This allows users to opt-out of string interning functionality.

Fixes #1434

Author: AArnott

src/StreamJsonRpc/MessagePackFormatter.cs

@@ -114,11 +114,26 @@ public class MessagePackFormatter : FormatterBase, IJsonRpcMessageFormatter, IJs
 
     private readonly ToStringHelper deserializationToStringHelper = new ToStringHelper();
 
+    /// <summary>
+    /// Backing field for the <see cref="InternStrings" /> property.
+    /// </summary>
+    private bool internStrings = true;
+
     /// <summary>
     /// The options to use for serializing user data (e.g. arguments, return values and errors).
     /// </summary>
     private MessagePackSerializerOptions userDataSerializationOptions;
 
+    /// <summary>
+    /// The original value supplied to <see cref="SetMessagePackSerializerOptions(MessagePackSerializerOptions)"/>
+    /// before we mutated it into <see cref="userDataSerializationOptions"/>.
+    /// </summary>
+    /// <remarks>
+    /// This value is useful when we have to rebuild the final serialization options
+    /// due to some other configuration change.
+    /// </remarks>
+    private MessagePackSerializerOptions originalUserDataSerializationOptions;
+
     /// <summary>
     /// Initializes a new instance of the <see cref="MessagePackFormatter"/> class.
     /// </summary>
@@ -136,6 +151,7 @@ public MessagePackFormatter()
         this.exceptionResolver = new MessagePackExceptionResolver(this);
 
         // Set up default user data resolver.
+        this.originalUserDataSerializationOptions = DefaultUserDataSerializationOptions;
         this.userDataSerializationOptions = this.MassageUserDataOptions(DefaultUserDataSerializationOptions);
     }
 
@@ -168,6 +184,28 @@ private interface IJsonRpcMessagePackRetention
         set => base.MultiplexingStream = value;
     }
 
+    /// <summary>
+    /// Gets or sets a value indicating whether user data should be deserialized
+    /// with string interning.
+    /// </summary>
+    /// <value>The default value is <see langword="true" />.</value>
+    public bool InternStrings
+    {
+        get => this.internStrings;
+        set
+        {
+            if (this.internStrings == value)
+            {
+                return;
+            }
+
+            this.internStrings = value;
+
+            // Reinitialize with the new setting.
+            this.userDataSerializationOptions = this.MassageUserDataOptions(this.originalUserDataSerializationOptions);
+        }
+    }
+
     /// <summary>
     /// Gets a value indicating whether the W3C <c>traceparent</c> property
     /// should be serialized as a string instead of a more compact binary format.
@@ -189,6 +227,7 @@ public void SetMessagePackSerializerOptions(MessagePackSerializerOptions options
     {
         Requires.NotNull(options, nameof(options));
 
+        this.originalUserDataSerializationOptions = options;
         this.userDataSerializationOptions = this.MassageUserDataOptions(options);
     }
 
@@ -340,14 +379,18 @@ private MessagePackSerializerOptions MassageUserDataOptions(MessagePackSerialize
         };
 
         // Add our own resolvers to fill in specialized behavior if the user doesn't provide/override it by their own resolver.
-        var resolvers = new IFormatterResolver[]
-        {
-            // Support for marshalled objects.
-            new RpcMarshalableResolver(this),
+        List<IFormatterResolver> resolvers =
+        [
+            new RpcMarshalableResolver(this), // Support for marshalled objects.
+        ];
 
-            // Intern strings to reduce memory usage.
-            StringInterningResolver,
+        if (this.InternStrings)
+        {
+            resolvers.Add(StringInterningResolver);
+        }
 
+        resolvers.AddRange(
+        [
             userSuppliedOptions.Resolver,
 
             // Add stateless, non-specialized resolvers that help basic functionality to "just work".
@@ -358,7 +401,7 @@ private MessagePackSerializerOptions MassageUserDataOptions(MessagePackSerialize
             this.asyncEnumerableFormatterResolver,
             this.pipeFormatterResolver,
             this.exceptionResolver,
-        };
+        ]);
 
         // Wrap the resolver in another class as a way to pass information to our custom formatters.
         IFormatterResolver userDataResolver = new ResolverWrapper(CompositeResolver.Create(formatters, resolvers), this);

test/StreamJsonRpc.Tests/MessagePackFormatterTests.cs

@@ -409,6 +409,55 @@ public void StringValuesOfStandardPropertiesAreInterned()
         Assert.Same(request1.Method, request2.Method); // reference equality to ensure it was interned.
     }
 
+    [Fact]
+    public void InternStrings_DefaultValueIsTrue()
+    {
+        MessagePackFormatter formatter = new();
+        Assert.True(formatter.InternStrings);
+    }
+
+    [Fact]
+    public void InternStrings_CanBeDisabledBeforeSetMessagePackSerializerOptions()
+    {
+        MessagePackFormatter formatter = new()
+        {
+            InternStrings = false,
+        };
+
+        Assert.False(this.AreTwoIdenticalStringsInterned(formatter));
+    }
+
+    [Fact]
+    public void InternStrings_CanBeDisabledAfterSetMessagePackSerializerOptions()
+    {
+        MessagePackFormatter formatter = new();
+        formatter.SetMessagePackSerializerOptions(MessagePackSerializerOptions.Standard);
+        formatter.InternStrings = false;
+
+        Assert.False(this.AreTwoIdenticalStringsInterned(formatter));
+    }
+
+    [Fact]
+    public void InternStrings_CanBeReEnabledBeforeSetMessagePackSerializerOptions()
+    {
+        MessagePackFormatter formatter = new();
+        formatter.InternStrings = false;
+        formatter.InternStrings = true;
+
+        Assert.True(this.AreTwoIdenticalStringsInterned(formatter));
+    }
+
+    [Fact]
+    public void InternStrings_CanBeReEnabledAfterSetMessagePackSerializerOptions()
+    {
+        MessagePackFormatter formatter = new();
+        formatter.InternStrings = false;
+        formatter.SetMessagePackSerializerOptions(MessagePackSerializerOptions.Standard);
+        formatter.InternStrings = true;
+
+        Assert.True(this.AreTwoIdenticalStringsInterned(formatter));
+    }
+
     protected override MessagePackFormatter CreateFormatter() => new();
 
     private T Read<T>(object anonymousObject)
@@ -421,6 +470,26 @@ private T Read<T>(object anonymousObject)
         return (T)this.Formatter.Deserialize(sequence);
     }
 
+    private bool AreTwoIdenticalStringsInterned(MessagePackFormatter formatter)
+    {
+        var anonymousObject = new
+        {
+            jsonrpc = "2.0",
+            method = "test",
+            @params = new object[] { "duplicate", "duplicate" },
+        };
+
+        var sequence = new Sequence<byte>();
+        var writer = new MessagePackWriter(sequence);
+        MessagePackSerializer.Serialize(ref writer, anonymousObject, MessagePackSerializerOptions.Standard);
+        writer.Flush();
+
+        var request = (JsonRpcRequest)formatter.Deserialize(sequence);
+        Assert.True(request.TryGetArgumentByNameOrIndex(null, 0, typeof(string), out object? arg1));
+        Assert.True(request.TryGetArgumentByNameOrIndex(null, 1, typeof(string), out object? arg2));
+        return ReferenceEquals(arg1, arg2);
+    }
+
     [DataContract]
     private class DataContractWithSubsetOfMembersIncluded
     {