Add System.BinaryData

[terrajobst]

@KrzysztofCwalina suggested to add a new type called BinaryData. The Azure SDK team found in user studies that people struggle with the various ways we represent binary data (span, memory, byte arrays, streams). They experimented with a new type BinaryData that is a struct-based wrapper around a byte[] and provides constructors, factory methods, and conversion methods to convert data from and to the various representations, including JSON serialization.

Full spec is here: dotnet/designs#155

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[terrajobst]

Video

• The type is very opinionated in order to achieve usability:
  • Serialization is done with System.Text.Json, with specific options
  • String encoding is always UTF8
• We're concerned with hard coding the type to System.Text.Json, but we acknowledge that defaults are very valuable. If we need to support other serializers, then we can add an enum
  • We could decide to make the constructor that performs JSON serialization as a factory method. Combined with a hypothetical feature extension statics we could make the type more low level than serialization
  • An alternative design is to replace constructors and factories with instance members (SetBytes, SetObject) which will throw when the underlying array is non-null. That prevents mutation and uses a simple pattern var x = new BinaryData(); x.SetBytes(...);. And on top, we can use regular extension methods to split out JSON serialization.
• This type can't live in corlib because it would depend on System.Text.Json (and whatever future serializers it needs to support). Hence, most areas in the BCL wouldn't be able to take BinaryData, but technologies above the BCL could (Microsoft.Extensions, ASP.NET Core, Azure SDK etc)
• Thus, we shouldn't think of this as an exchange type in the BCL. We already have exchange types; the point of this type is unify them by providing conversions; the point isn't to provide yet another type. Thus, we don't see the need to, for example, provide new virtual methods on Stream.

namespace System
{
    public readonly struct BinaryData
    {
        public BinaryData(ReadOnlySpan<byte> data);
        public BinaryData(byte[] data);
        public BinaryData(object jsonSerializable, Type? type = null);
        public BinaryData(ReadOnlyMemory<byte> data);
        public BinaryData(string data);
        public static BinaryData FromBytes(ReadOnlyMemory<byte> data);
        public static BinaryData FromBytes(ReadOnlySpan<byte> data);
        public static BinaryData FromBytes(byte[] data);
        public static BinaryData FromObject<T>(T jsonSerializable, CancellationToken cancellationToken = default);
        public static Task<BinaryData> FromObjectAsync<T>(T jsonSerializable, CancellationToken cancellationToken = default);
        public static BinaryData FromStream(Stream stream);
        public static Task<BinaryData> FromStreamAsync(Stream stream, CancellationToken cancellationToken = default);
        public static BinaryData FromString(string data);
        public static implicit operator ReadOnlyMemory<byte>(BinaryData data);
        public ReadOnlyMemory<byte> ToBytes();
        public T ToObject<T>(CancellationToken cancellationToken = default);
        public ValueTask<T> ToObjectAsync<T>(CancellationToken cancellationToken = default);
        public Stream ToStream();
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override bool Equals(object? obj);
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override int GetHashCode();
        public override string ToString();
    }
}

[Wraith2]

I have a similar type in my own libraries and one thing I've found useful which isn't in this surface is base64 read and write capability.

[terrajobst]

• Spec: Initial draft of spec for System.BinaryData designs#155
• We believe there are usability issue with instance methods as proposed in the spec.
• We should consider exposing an ObjectSerializer abstraction, like Azure SDK has. This would solve the 3rd party extension problem in general.
• Using an enum as a format will be harder to in static linking
• Current POR:
  • Leave constructors
  • Leave statics
  • Consider suffixing FromObject and ToObject

[carlreinke]

Regarding constructors vs factory methods, perhaps future tooling could suggest FactoryAttribute-ed static methods from a type when the user types new Whatever. (FactoryAttribute is/was being considered for use in a future version of C# where the compiler would require that an attributed method return a new instance.) Or even without FactoryAttribute, tooling could suggest static methods that return the containing type when the user types new Whatever.

[dersia]

I didn't see @carlreinke comment when I suggested #42314 but I think this is exactly what you were referring to

[terrajobst]

Video

• The type isn't meant to unify different representations
• Rather, it's a helper to make it easier for users to convert data
• Thus, it's not meant as high-performance exchange type, like span.
• It's for "user data", that is, the contract for the API is "give form blob of binary data". It's not for protocol-like APIs where the underlying encoding is part of the API contract, e.g. a REST API.
  • This means the API is meant for things like Azure Blob Storage, Azure Queuing, but not HttpClient
• We should consider making the type a class. It's unlike to have perf implications (given that constructing it generally requires copying)
  • This also gives us flexibility in the future, e.g. sub-classing
  • Not much value in sealing (we only give up not being able to add abstracts later)
• The methods that perform JSON serialization/deserialization should take JSON options
  • We should make this the second parameter so we can add new serializers where type remains optional
• We can remove the FromObjectAsync and ToObjectAsync methods, they are hold over from the serialization abstraction which had async implementations
• We should remove the constructor and factory that takes a span, because it copies and it collides wit the ReadOnlyMemory<T>
• We should add an implicit conversion to span (guidelines say that one shouldn't overload between them, so it seems generally fine)
• We should consider hiding Equals(), GetHashCode()
• The constructors that take byte[] and ReadOnlyMemory<byte> will not copy; they just wrap the payload. Mutations will be observed.

namespace System
{
    public class BinaryData
    {
        public BinaryData(byte[] data);
        public BinaryData(object jsonSerializable, JsonSerializerOptions options = default, Type? type = null);
        public BinaryData(ReadOnlyMemory<byte> data);
        public BinaryData(string data);
        public static BinaryData FromBytes(ReadOnlyMemory<byte> data);
        public static BinaryData FromBytes(byte[] data);
        public static BinaryData FromObjectAsJson<T>(T jsonSerializable, JsonSerializerOptions options = default, CancellationToken cancellationToken = default);
        public static BinaryData FromStream(Stream stream);
        public static Task<BinaryData> FromStreamAsync(Stream stream, CancellationToken cancellationToken = default);
        public static BinaryData FromString(string data);
        public static implicit operator ReadOnlyMemory<byte>(BinaryData data);
        public static implicit operator ReadOnlySpan<byte>(BinaryData data);
        public ReadOnlyMemory<byte> ToBytes();
        public T ToObjectFromJson<T>(JsonSerializerOptions options = default, CancellationToken cancellationToken = default);
        public Stream ToStream();
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override bool Equals(object? obj);
        [EditorBrowsable(EditorBrowsableState.Never)]
        public override int GetHashCode();
        public override string ToString();
    }
}

[ReubenBond]

I believe this should support ReadOnlySequence<byte>, too:

namespace System
{
    public class BinaryData
    {
        public BinaryData(ReadOnlySequence<byte> data);
        public static BinaryData FromBytes(ReadOnlySequence<byte> data);
    }
}

[GSPP]

It seems like a layering violation that...

• ...the namespace System refers to JSON as a concept
• ...a type that is about representing binary data has ties to a specific serialization format (JSON)

As it stands, this type appears to be a DTO helper object hard-coded to use System.Text.Json. It would more cleanly fit into that namespace and package.

[stephentoub]

@terrajobst, with https://github.com/dotnet/runtime/tree/8a52f1e948b6f22f418817ec1068f07b8dae2aa5/src/libraries/System.Memory.Data, can this be closed now?

[terrajobst]

Yep