feat(storage): add System.Text.Json grain storage serializer - #9505
Conversation
1f0fc06 to
58de1dd
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds System.Text.Json support for grain state persistence in Orleans, providing a performance-optimized alternative to Newtonsoft.Json. The implementation maintains backward compatibility by using the same serialization structure as Newtonsoft, enabling seamless migration without breaking existing persisted state.
Changes:
- Adds System.Text.Json converters for 15+ Orleans types (StreamId, GrainId, ActivationId, SiloAddress, etc.) with backward compatibility with Newtonsoft.Json format
- Implements IUtf8SpanFormattable on StreamId and extends existing converters with property name serialization support for dictionary key scenarios
- Introduces SystemTextJsonGrainStorageSerializer as an experimental feature behind ORLEANSEXP006 flag, allowing opt-in adoption
Reviewed changes
Copilot reviewed 22 out of 26 changed files in this pull request and generated 19 comments.
Show a summary per file
| File | Description |
|---|---|
| test/TesterInternal/StorageTests/SystemTextJsonStorageSerializer.cs | Comprehensive test suite verifying round-trip serialization compatibility between STJ and Newtonsoft |
| src/Orleans.Streaming/StreamId.cs | Adds JsonConverter attribute, IUtf8SpanFormattable implementation, and StreamIdJsonConverter with backward compatibility |
| src/Orleans.Streaming/PubSub/PubSubSubscriptionState.cs | Adds System.Text.Json attributes and JsonConstructor for STJ support |
| src/Orleans.Streaming/JsonConverters/* | Adds STJ converters for streaming types (EventSequenceToken, AsyncStream, QualifiedStreamId) |
| src/Orleans.Streaming/InternalStreamId.cs | Adds QualifiedStreamIdJsonConverter with property name serialization support |
| src/Orleans.Streaming/Hosting/StreamingServiceCollectionExtensions.cs | Registers STJ converter configurator in DI |
| src/Orleans.Runtime/Hosting/SystemTextJsonSerializerExtensions.cs | Extension method to enable STJ as default grain storage serializer (experimental) |
| src/Orleans.Runtime/Hosting/DefaultSiloServices.cs | Registers STJ serializer options and configurator in default services |
| src/Orleans.Core/Serialization/SystemTextJson/* | Core STJ converters for IP addresses, endpoints, GrainId, and GrainReference |
| src/Orleans.Core/Serialization/SystemTextJson/SystemTextJsonGrainStorageSerializerOptions.cs | Configuration options for STJ serializer with default settings |
| src/Orleans.Core/Serialization/NewtonSoft/* | Refactored Newtonsoft.Json serialization code into separate organized files |
| src/Orleans.Core/Providers/StorageSerializer/SystemTextJsonGrainStorageSerializer.cs | Main STJ serializer implementation using BinaryData |
| src/Orleans.Core/Providers/StorageSerializer/JsonGrainStorageSerializer.cs | Minor change to seal the class |
| src/Orleans.Core.Abstractions/Runtime/MembershipVersion.cs | Adds property name serialization support to existing converter |
| src/Orleans.Core.Abstractions/IDs/SiloAddress.cs | Adds property name serialization support to existing converter |
| src/Orleans.Core.Abstractions/IDs/Legacy/UniqueKey.cs | Adds JsonConverter attribute and UniqueKeyJsonConverter |
| src/Orleans.Core.Abstractions/IDs/GuidId.cs | Adds JsonConverter attribute and GuidIdConverter with property name support |
| src/Orleans.Core.Abstractions/IDs/ActivationId.cs | Extends existing converter with property name serialization support |
c5196cf to
1e05bf0
Compare
1e05bf0 to
732701f
Compare
9981ebf to
72f04a8
Compare
There was a problem hiding this comment.
Review details
Suppressed comments (5)
src/Orleans.Streaming/StreamId.cs:333
- StreamIdJsonConverter.Read also needs to explicitly reject "$ref" inside the "fk" object to avoid silently returning default(StreamId) when Newtonsoft emits reference metadata.
if (reader.TokenType == JsonTokenType.PropertyName)
{
propertyName = reader.GetString();
reader.Read();
if (propertyName == "$value")
{
src/Orleans.Streaming/JsonConverters/AsyncStreamConverter.cs:74
- AsyncStreamConverter.Read assumes the target type is generic and calls MakeGenericType(typeToConvert.GetGenericArguments()). If the state member is typed as non-generic IAsyncStream, GetGenericArguments() is empty and this throws at runtime with an unhelpful exception. Prefer failing explicitly with a JsonException (or otherwise handling the non-generic case) and also validate that the resolved provider implements IInternalStreamProvider.
return (IAsyncStream)Activator.CreateInstance(
typeof(StreamImpl<>).MakeGenericType(typeToConvert.GetGenericArguments()),
new QualifiedStreamId(providerName, streamId.Value),
src/Orleans.Core/Serialization/SystemTextJson/SystemTextJsonGrainStorageSerializerOptions.cs:23
- SystemTextJsonGrainStorageSerializerOptions enables WriteIndented=true by default, which increases payload size and allocations for grain state persistence. Newtonsoft grain storage defaults to Formatting.None (OrleansJsonSerializerSettings.GetDefaultSerializerSettings), so this is also inconsistent with existing behavior; consider defaulting to non-indented JSON and letting callers opt-in via options if they want readability.
{
WriteIndented = true,
IncludeFields = true,
DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
};
test/Orleans.Runtime.Internal.Tests/StorageTests/SystemTextJsonStorageSerializer.cs:57
- This test uses DeployAsync().Wait(), which can wrap exceptions in AggregateException and is inconsistent with the rest of the test suite (which typically uses GetAwaiter().GetResult()). Use GetAwaiter().GetResult() to preserve the original exception and avoid sync-context deadlock pitfalls.
_testCluster.DeployAsync().Wait();
src/Orleans.Streaming/JsonConverters/EventSequenceTokenJsonConverter.cs:46
- EventSequenceTokenJsonConverter.Read ignores unexpected properties. If Newtonsoft emits reference metadata ("$ref") due to PreserveReferencesHandling.Objects, this converter would currently return null, potentially masking data issues. Consider explicitly rejecting "$ref" with a JsonException (consistent with GuidIdConverter) to fail fast instead of silently dropping the value.
if (reader.TokenType == JsonTokenType.PropertyName)
{
var propertyName = reader.GetString();
reader.Read();
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Lite
|
Addressed the suppressed review findings in 6a83779: |
There was a problem hiding this comment.
Review details
Suppressed comments (8)
src/Orleans.Core/Serialization/SystemTextJson/GrainReferenceConverter.cs:35
GrainReferenceConverter.Readwill silently ignore a top-level$refpayload (possible with Orleans' Newtonsoft settings usingPreserveReferencesHandling.Objects) and returnnull, causing silent data loss/corruption. Reject$refexplicitly.
if (reader.TokenType == JsonTokenType.PropertyName)
{
var propertyName = reader.GetString();
reader.Read();
src/Orleans.Streaming/InternalStreamId.cs:90
QualifiedStreamIdJsonConverter.Readdoes not reject a$refpayload, which can be emitted by Orleans' Newtonsoft serializer when reference preservation is enabled. Reject$refexplicitly to avoid silent corruption.
if (reader.TokenType == JsonTokenType.PropertyName)
{
var propertyName = reader.GetString();
reader.Read();
src/Orleans.Streaming/JsonConverters/AsyncStreamConverter.cs:43
AsyncStreamConverter.Readdoes not reject a$refpayload (possible with Orleans' Newtonsoft settings using reference preservation), which would currently be ignored and likely producenull. Reject$refexplicitly.
if (reader.TokenType == JsonTokenType.PropertyName)
{
var propertyName = reader.GetString()!;
reader.Read();
switch (propertyName)
test/Orleans.Runtime.Internal.Tests/StorageTests/SystemTextJsonStorageSerializer.cs:27
IAdditionalInterface.GetAltis implemented as a default interface method, so calls on a grain reference can execute locally instead of being dispatched as an Orleans RPC. That makes this test ineffective for validating that serialized grain references preserve interface identity/dispatch.
Make GetAlt abstract on the interface and implement it on ReferenceTesterGrain.
interface IAdditionalInterface : IGrainWithGuidKey
{
public ValueTask<int> GetAlt() => ValueTask.FromResult(731131);
}
src/Orleans.Core/Serialization/SystemTextJson/GrainReferenceConverter.cs:26
GrainReferenceConverter.Readdoesn't validate the initial token type. If the payload isnullor not a JSON object, the current loop will advance the reader and may returnnullsilently.
Add an explicit token check (allowing null to map to null) and throw JsonException otherwise.
This issue also appears on line 31 of the same file.
public override IAddressable? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
string? type = null, key = null, iface = null;
while (reader.Read())
src/Orleans.Streaming/InternalStreamId.cs:74
QualifiedStreamIdJsonConverter.Readreturnsdefaultfor unexpected token types, which can silently turn malformed data into a valid-lookingQualifiedStreamId. Prefer throwing on unexpected tokens (allowingnullonly if you intentionally want it to map todefault).
This issue also appears on line 86 of the same file.
public override QualifiedStreamId Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.StartObject)
{
return default;
}
src/Orleans.Streaming/JsonConverters/AsyncStreamConverter.cs:26
AsyncStreamConverter.Readreturnsnullwhen the JSON token is not an object, which can silently drop invalid/malformed persisted data. Treatnullasnull, but throw for other unexpected token types.
This issue also appears on line 39 of the same file.
public override IAsyncStream? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
if (reader.TokenType != JsonTokenType.StartObject)
{
return null;
}
src/Orleans.Streaming/StreamId.cs:300
StreamIdJsonConverter.Readreturnsdefault(StreamId)when the token is not an object. For persisted grain state this can silently turn malformed/unexpected data into a default value; prefer throwingJsonExceptionfor unexpected tokens (allowingnullonly if you intentionally want it to map todefault).
if (reader.TokenType != JsonTokenType.StartObject)
{
return default;
}
- Files reviewed: 25/25 changed files
- Comments generated: 1
- Review effort level: Lite
There was a problem hiding this comment.
Review details
Suppressed comments (7)
src/Orleans.Core/Serialization/SystemTextJson/GrainIdJsonConverter.cs:55
- GrainIdJsonConverter.Read returns default(GrainId) when required Type/Key properties are missing. Newtonsoft's GrainIdConverter will throw in this case, so returning default here can hide corrupted payloads. Throw JsonException instead of returning default when Type/Key are absent.
if (string.IsNullOrWhiteSpace(type) || string.IsNullOrWhiteSpace(key))
{
return default;
}
else
{
return GrainId.Create(type, key);
}
src/Orleans.Streaming/StreamId.cs:293
- StreamIdJsonConverter writes $type and fk.$type using AssemblyQualifiedName, which includes version/culture/public key token. Orleans' Newtonsoft storage serializer uses TypeNameAssemblyFormatHandling.Simple, so versioned type names can make persisted state fragile across Orleans/.NET version changes (and differs from the legacy JSON shape). Emit simple assembly-qualified names instead ("{FullName}, {AssemblyName}").
private readonly string? _byteArrayType = typeof(byte[]).AssemblyQualifiedName;
private readonly string? _streamIdType = typeof(StreamId).AssemblyQualifiedName;
src/Orleans.Streaming/StreamId.cs:300
- StreamIdJsonConverter.Read returns default(StreamId) for any non-object token, which can silently convert malformed JSON into a valid value. Prefer treating null explicitly and throwing JsonException for other unexpected token types to avoid hidden data loss/corruption.
if (reader.TokenType != JsonTokenType.StartObject)
{
return default;
}
src/Orleans.Streaming/JsonConverters/EventSequenceTokenJsonConverter.cs:96
- EventSequenceTokenJsonConverter writes $type using AssemblyQualifiedName (includes version/culture/public key token). Orleans' Newtonsoft serializer uses TypeNameAssemblyFormatHandling.Simple, so this can make persisted state brittle across version changes. Write a simple assembly-qualified name instead.
writer.WriteString("$type", runtimeType.AssemblyQualifiedName); // For backward compatibility with Newtonsoft
src/Orleans.Streaming/InternalStreamId.cs:65
- QualifiedStreamIdJsonConverter writes $type using AssemblyQualifiedName (includes version/culture/public key token). Orleans' Newtonsoft storage serializer uses TypeNameAssemblyFormatHandling.Simple, so versioned type names can reduce cross-version compatibility of persisted state. Emit a simple assembly-qualified name instead.
private readonly string? _qualifiedStreamIdType = typeof(QualifiedStreamId).AssemblyQualifiedName;
src/Orleans.Streaming/JsonConverters/AsyncStreamConverter.cs:54
- AsyncStreamConverter.Read does not skip unknown properties. If an unknown property has an object/array value, the reader will descend into it and misinterpret nested tokens as top-level properties. Also, a top-level $ref payload currently deserializes to null instead of failing explicitly. Add a default case which calls reader.Skip(), and reject $ref.
var propertyName = reader.GetString()!;
reader.Read();
switch (propertyName)
{
case "streamId":
src/Orleans.Core/Serialization/SystemTextJson/GrainIdJsonConverter.cs:22
- GrainIdJsonConverter.Read returns default(GrainId) for non-object tokens, silently converting malformed JSON into a valid value. Handle null explicitly and throw JsonException for other unexpected token types (optionally accepting the existing string form via GrainId.Parse for resilience).
This issue also appears on line 48 of the same file.
if (reader.TokenType != JsonTokenType.StartObject)
{
return default;
}
- Files reviewed: 25/25 changed files
- Comments generated: 0 new
- Review effort level: Lite
c94a08a to
3f05bbd
Compare
There was a problem hiding this comment.
Review details
Suppressed comments (7)
src/Orleans.Core/Serialization/SystemTextJson/GrainReferenceConverter.cs:37
GrainReferenceConverter.Readignores "$ref" and does not skip unknown property values. That can silently produce null for "$ref" payloads and can allow nested JSON to be interpreted as top-level properties during parsing.
var propertyName = reader.GetString();
reader.Read();
if (propertyName == "Id")
{
src/Orleans.Streaming/StreamId.cs:393
- Legacy dictionary-key values produced by the previous
StreamId.ToString()used a "null/" prefix for null namespaces.ParsePropertyNamecurrently routes legacy keys directly toStreamId.Parse, which will interpret "null" as a real namespace and break backward reads during migration.
if (!value.StartsWith(PropertyNamePrefix, StringComparison.Ordinal)
|| value.AsSpan(PropertyNamePrefix.Length).Contains('/'))
{
return StreamId.Parse(Encoding.UTF8.GetBytes(value));
}
src/Orleans.Streaming/InternalStreamId.cs:158
- Legacy QualifiedStreamId dictionary keys can contain StreamId keys which used the old "null/" prefix for null namespaces. Parsing the stream id portion via
StreamId.Parse(...)will treat "null" as a real namespace and break backward reads during migration.
var providerName = value[..i];
var streamId = StreamId.Parse(Encoding.UTF8.GetBytes(value[(i + 1)..]));
return new QualifiedStreamId(providerName, streamId);
src/Orleans.Core/Serialization/SystemTextJson/GrainReferenceConverter.cs:25
GrainReferenceConverter.Readdoes not validate the initial token type and will currently return null for non-object payloads, making malformed JSON and reference-preserving payloads fail silently instead of explicitly.
This issue also appears on line 33 of the same file.
public override IAddressable? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
string? type = null, key = null, iface = null;
src/Orleans.Streaming/JsonConverters/AsyncStreamConverter.cs:55
AsyncStreamConverter.Readdoes not reject Newtonsoft reference-preserving payloads ("$ref") and does not skip unknown property values. That can silently return null for "$ref" payloads and can allow nested objects to be misinterpreted as top-level properties.
var propertyName = reader.GetString()!;
reader.Read();
switch (propertyName)
{
case "streamId":
src/Orleans.Core/Serialization/SystemTextJson/GrainIdJsonConverter.cs:45
GrainIdJsonConverter.Readcurrently ignores "$ref" and does not skip unknown property values, so reference-preserving payloads can be silently interpreted asdefault(GrainId)and nested objects can be misread as top-level properties.
var propertyName = reader.GetString();
reader.Read();
switch (propertyName)
{
src/Orleans.Core.Abstractions/IDs/Legacy/UniqueKey.cs:384
UniqueKeyJsonConverter.Readcurrently ignores "$ref" payloads and will return null instead of rejecting reference-preserving JSON explicitly, which can lead to silent data loss when migrating from Newtonsoft settings which emit "$ref".
if (reader.TokenType == JsonTokenType.PropertyName)
{
var isUniqueKey = reader.ValueTextEquals("UniqueKey");
if (!reader.Read())
{
- Files reviewed: 25/25 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
Review details
Suppressed comments (5)
Previously missed (1) — in code that hasn't changed since the last review.
src/Orleans.Core/Serialization/SystemTextJson/GrainReferenceConverter.cs:26
- GrainReferenceConverter.Read does not validate the initial token type. Non-object JSON will currently be consumed and result in null, which can silently drop persisted references instead of failing fast.
This issue also appears on line 36 of the same file.
public override IAddressable? Read(ref Utf8JsonReader reader, Type typeToConvert, JsonSerializerOptions options)
{
string? type = null, key = null, iface = null;
while (reader.Read())
src/Orleans.Streaming/StreamId.cs:351
- StreamIdJsonConverter.Read casts the JSON "ki" value from uint to ushort without validating bounds. Out-of-range values will wrap and can produce an invalid StreamId which does not roundtrip the original persisted data.
return value is not { Length: > 0 }
|| !ki.HasValue
? default
: new StreamId(value, (ushort)ki);
src/Orleans.Streaming/StreamId.cs:392
- StreamIdJsonConverter.ParsePropertyName falls back to StreamId.Parse for legacy dictionary keys. Prior StreamId string formatting used the "null/" prefix for a null namespace, which StreamId.Parse interprets as a literal "null" namespace (breaking backward reads of those keys).
if (!value.StartsWith(PropertyNamePrefix, StringComparison.Ordinal)
|| value.AsSpan(PropertyNamePrefix.Length).Contains('/'))
{
return StreamId.Parse(Encoding.UTF8.GetBytes(value));
}
src/Orleans.Streaming/InternalStreamId.cs:153
- QualifiedStreamIdJsonConverter.ReadAsPropertyName only supports the legacy "provider:namespace/key" form. QualifiedStreamId.ToString() uses '/' ("provider/namespace/key"), which is also what Newtonsoft.Json dictionary-key serialization commonly uses for non-string keys; that form currently throws during migration.
var i = value.IndexOf(':');
if (i < 0)
{
throw new JsonException("Failed to parse QualifiedStreamId from property name.");
src/Orleans.Core/Serialization/SystemTextJson/GrainReferenceConverter.cs:36
- GrainReferenceConverter.Read does not explicitly reject reference-preserving "$ref" payloads. If a persisted grain reference is emitted as {"$ref":"..."}, this converter will currently return null (silent data loss) instead of failing explicitly as described in the PR.
if (propertyName == "Id")
- Files reviewed: 25/25 changed files
- Comments generated: 2
- Review effort level: Lite
Problem
Orleans grain storage uses Newtonsoft.Json by default. Applications which prefer System.Text.Json need an integrated storage serializer, including support for Orleans runtime types commonly found in persisted state.
Solution
Add
SystemTextJsonGrainStorageSerializerand the opt-inUseSystemTextJsonGrainStorageSerializer()silo-builder extension.The serializer supports Orleans activation, grain, silo, membership, stream, subscription, endpoint, and grain-reference types. It establishes compact canonical formats for new persisted state: scalar values use strings or numbers, fixed-arity composite values use arrays, and optional subscription state uses a camelCase object. Stream dictionary keys use length-prefixed components to preserve arbitrary namespaces, keys, and provider names without delimiter ambiguity.
Converters validate token shapes, required components, UTF-8 content, and supported runtime types. System.Text.Json treats legacy
$typemetadata as ordinary data, while converter discriminators select only fixed Orleans types.Rationale
System.Text.Json provides a modern, lower-allocation serialization option with explicit canonical storage contracts. Keeping runtime identity strings separate from JSON formats preserves Orleans routing and pub/sub identities while allowing storage formats to remain compact and unambiguous.
Enable it with:
Applications can customize the underlying
JsonSerializerOptionsthroughSystemTextJsonGrainStorageSerializerOptions.