Skip to content

Commit 800b793

Browse files
committed
Fix JetStream API serialization with typed results
Replace JsonDocument-based responses with strongly-typed `NatsJSApiResult<T>` for improved type safety and error handling. Added new deserialization logic and tests to cover valid responses, errors, and edge cases like empty buffers.
1 parent e100805 commit 800b793

File tree

4 files changed

+127
-27
lines changed

4 files changed

+127
-27
lines changed
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
using System.Runtime.CompilerServices;
2+
using NATS.Client.JetStream.Models;
3+
4+
namespace NATS.Client.JetStream.Internal;
5+
6+
internal readonly struct NatsJSApiResult<T>
7+
{
8+
private readonly T? _value;
9+
private readonly ApiError? _error;
10+
private readonly Exception? _exception;
11+
12+
public NatsJSApiResult(T value)
13+
{
14+
_value = value;
15+
_error = null;
16+
_exception = null;
17+
}
18+
19+
public NatsJSApiResult(ApiError error)
20+
{
21+
_value = default;
22+
_error = error;
23+
_exception = null;
24+
}
25+
26+
public NatsJSApiResult(Exception exception)
27+
{
28+
_value = default;
29+
_error = null;
30+
_exception = exception;
31+
}
32+
33+
public T Value => _value ?? ThrowValueIsNotSetException();
34+
35+
public ApiError Error => _error ?? ThrowErrorIsNotSetException();
36+
37+
public Exception Exception => _exception ?? ThrowExceptionIsNotSetException();
38+
39+
public bool Success => _error == null && _exception == null;
40+
41+
public bool HasError => _error != null;
42+
43+
public bool HasException => _exception != null;
44+
45+
public static implicit operator NatsJSApiResult<T>(T value) => new(value);
46+
47+
public static implicit operator NatsJSApiResult<T>(ApiError error) => new(error);
48+
49+
public static implicit operator NatsJSApiResult<T>(Exception exception) => new(exception);
50+
51+
private static T ThrowValueIsNotSetException() => throw CreateInvalidOperationException("Result value is not set");
52+
53+
private static ApiError ThrowErrorIsNotSetException() => throw CreateInvalidOperationException("Result error is not set");
54+
55+
private static Exception ThrowExceptionIsNotSetException() => throw CreateInvalidOperationException("Result exception is not set");
56+
57+
[MethodImpl(MethodImplOptions.NoInlining)]
58+
private static Exception CreateInvalidOperationException(string message) => new InvalidOperationException(message);
59+
}

src/NATS.Client.JetStream/Internal/NatsJSJsonDocumentSerializer.cs

+32-3
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,38 @@
44

55
namespace NATS.Client.JetStream.Internal;
66

7-
internal sealed class NatsJSJsonDocumentSerializer : INatsDeserialize<JsonDocument>
7+
internal sealed class NatsJSJsonDocumentSerializer<T> : INatsDeserialize<NatsJSApiResult<T>>
88
{
9-
public static readonly NatsJSJsonDocumentSerializer Default = new();
9+
public static readonly NatsJSJsonDocumentSerializer<T> Default = new();
1010

11-
public JsonDocument? Deserialize(in ReadOnlySequence<byte> buffer) => buffer.Length == 0 ? default : JsonDocument.Parse(buffer);
11+
public NatsJSApiResult<T> Deserialize(in ReadOnlySequence<byte> buffer)
12+
{
13+
if (buffer.Length == 0)
14+
{
15+
return new NatsJSException("Buffer is empty");
16+
}
17+
18+
using var jsonDocument = JsonDocument.Parse(buffer);
19+
20+
if (jsonDocument.RootElement.TryGetProperty("error", out var errorElement))
21+
{
22+
var error = errorElement.Deserialize(JetStream.NatsJSJsonSerializerContext.Default.ApiError) ?? throw new NatsJSException("Can't parse JetStream error JSON payload");
23+
return error;
24+
}
25+
26+
var jsonTypeInfo = NatsJSJsonSerializerContext.DefaultContext.GetTypeInfo(typeof(T));
27+
if (jsonTypeInfo == null)
28+
{
29+
return new NatsJSException($"Unknown response type {typeof(T)}");
30+
}
31+
32+
var result = (T?)jsonDocument.RootElement.Deserialize(jsonTypeInfo);
33+
34+
if (result == null)
35+
{
36+
return new NatsJSException("Null result");
37+
}
38+
39+
return result;
40+
}
1241
}

src/NATS.Client.JetStream/NatsJSContext.cs

+9-24
Original file line numberDiff line numberDiff line change
@@ -309,13 +309,13 @@ internal async ValueTask<NatsResult<NatsJSResponse<TResponse>>> TryJSRequestAsyn
309309
// Validator.ValidateObject(request, new ValidationContext(request));
310310
}
311311

312-
await using var sub = await Connection.CreateRequestSubAsync<TRequest, JsonDocument>(
312+
await using var sub = await Connection.CreateRequestSubAsync<TRequest, NatsJSApiResult<TResponse>>(
313313
subject: subject,
314314
data: request,
315315
headers: default,
316316
replyOpts: new NatsSubOpts { Timeout = Connection.Opts.RequestTimeout },
317317
requestSerializer: NatsJSJsonSerializer<TRequest>.Default,
318-
replySerializer: NatsJSJsonDocumentSerializer.Default,
318+
replySerializer: NatsJSJsonDocumentSerializer<TResponse>.Default,
319319
cancellationToken: cancellationToken)
320320
.ConfigureAwait(false);
321321

@@ -326,37 +326,22 @@ internal async ValueTask<NatsResult<NatsJSResponse<TResponse>>> TryJSRequestAsyn
326326
return new NatsNoRespondersException();
327327
}
328328

329-
if (msg.Data == null)
330-
{
331-
return new NatsJSException("No response data received");
332-
}
333-
334-
// We need to determine what type we're deserializing into
335-
// .NET 6 new APIs to the rescue: we can read the buffer once
336-
// by deserializing into a document, inspect and using the new
337-
// API deserialize to the final type from the document.
338-
using var jsonDocument = msg.Data;
339-
340-
if (jsonDocument.RootElement.TryGetProperty("error", out var errorElement))
329+
if (msg.Error is { } messageError)
341330
{
342-
var error = errorElement.Deserialize(JetStream.NatsJSJsonSerializerContext.Default.ApiError) ?? throw new NatsJSException("Can't parse JetStream error JSON payload");
343-
return new NatsJSResponse<TResponse>(default, error);
331+
return messageError;
344332
}
345333

346-
var jsonTypeInfo = NatsJSJsonSerializerContext.DefaultContext.GetTypeInfo(typeof(TResponse));
347-
if (jsonTypeInfo == null)
334+
if (msg.Data.HasException)
348335
{
349-
return new NatsJSException($"Unknown response type {typeof(TResponse)}");
336+
return msg.Data.Exception;
350337
}
351338

352-
var response = (TResponse?)jsonDocument.RootElement.Deserialize(jsonTypeInfo);
353-
354-
if (msg.Error is { } messageError)
339+
if (msg.Data.HasError)
355340
{
356-
return messageError;
341+
return new NatsJSResponse<TResponse>(null, msg.Data.Error);
357342
}
358343

359-
return new NatsJSResponse<TResponse>(response, default);
344+
return new NatsJSResponse<TResponse>(msg.Data.Value, null);
360345
}
361346

362347
if (sub is NatsSubBase { EndReason: NatsSubEndReason.Exception, Exception: not null } sb)

tests/NATS.Client.JetStream.Tests/JetStreamApiSerializerTest.cs

+27
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,7 @@
1+
using System.Buffers;
2+
using System.Text;
13
using NATS.Client.Core2.Tests;
4+
using NATS.Client.JetStream.Internal;
25
using NATS.Client.JetStream.Models;
36
using JsonSerializer = System.Text.Json.JsonSerializer;
47

@@ -98,4 +101,28 @@ public async Task Should_respect_buffers_lifecycle()
98101
{
99102
}
100103
}
104+
105+
[Fact]
106+
public void Deserialize_value()
107+
{
108+
var serializer = NatsJSJsonDocumentSerializer<AccountInfoResponse>.Default;
109+
var result = serializer.Deserialize(new ReadOnlySequence<byte>(Encoding.UTF8.GetBytes("""{"memory":1}""")));
110+
result.Value.Memory.Should().Be(1);
111+
}
112+
113+
[Fact]
114+
public void Deserialize_empty_buffer()
115+
{
116+
var serializer = NatsJSJsonDocumentSerializer<AccountInfoResponse>.Default;
117+
var result = serializer.Deserialize(ReadOnlySequence<byte>.Empty);
118+
result.Exception.Message.Should().Be("Buffer is empty");
119+
}
120+
121+
[Fact]
122+
public void Deserialize_error()
123+
{
124+
var serializer = NatsJSJsonDocumentSerializer<AccountInfoResponse>.Default;
125+
var result = serializer.Deserialize(new ReadOnlySequence<byte>(Encoding.UTF8.GetBytes("""{"error":{"code":2}}""")));
126+
result.Error.Code.Should().Be(2);
127+
}
101128
}

0 commit comments

Comments
 (0)