Skip to content

Address JS to DotNet byte[] Interop API Review Feedback #34328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions src/Components/Server/src/Circuits/RemoteJSDataStream.cs
Original file line number Diff line number Diff line change
Expand Up @@ -40,22 +40,20 @@ public static async ValueTask<RemoteJSDataStream> CreateRemoteJSDataStreamAsync(
RemoteJSRuntime runtime,
IJSStreamReference jsStreamReference,
long totalLength,
long maximumIncomingBytes,
long signalRMaximumIncomingBytes,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change wasn't part of the API review, but I just felt this updated name could avoid future confusion (ex. clarifying it's not the max incoming bytes for the whole stream somehow).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great clarification. I probably would have thought it referred to the stream length before :)

TimeSpan jsInteropDefaultCallTimeout,
long pauseIncomingBytesThreshold = -1,
long resumeIncomingBytesThreshold = -1,
CancellationToken cancellationToken = default)
{
// Enforce minimum 1 kb, maximum 50 kb, SignalR message size.
// We budget 512 bytes overhead for the transfer, thus leaving at least 512 bytes for data
// transfer per chunk with a 1 kb message size.
// Additionally, to maintain interactivity, we put an upper limit of 50 kb on the message size.
var chunkSize = maximumIncomingBytes > 1024 ?
Math.Min(maximumIncomingBytes, 50*1024) - 512 :
var chunkSize = signalRMaximumIncomingBytes > 1024 ?
Math.Min(signalRMaximumIncomingBytes, 50*1024) - 512 :
throw new ArgumentException($"SignalR MaximumIncomingBytes must be at least 1 kb.");

var streamId = runtime.RemoteJSDataStreamNextInstanceId++;
var remoteJSDataStream = new RemoteJSDataStream(runtime, streamId, totalLength, jsInteropDefaultCallTimeout, pauseIncomingBytesThreshold, resumeIncomingBytesThreshold, cancellationToken);
var remoteJSDataStream = new RemoteJSDataStream(runtime, streamId, totalLength, jsInteropDefaultCallTimeout, cancellationToken);
await runtime.InvokeVoidAsync("Blazor._internal.sendJSDataStream", jsStreamReference, streamId, chunkSize);
return remoteJSDataStream;
}
Expand All @@ -65,8 +63,6 @@ private RemoteJSDataStream(
long streamId,
long totalLength,
TimeSpan jsInteropDefaultCallTimeout,
long pauseIncomingBytesThreshold,
long resumeIncomingBytesThreshold,
CancellationToken cancellationToken)
{
_runtime = runtime;
Expand All @@ -80,7 +76,7 @@ private RemoteJSDataStream(

_runtime.RemoteJSDataStreamInstances.Add(_streamId, this);

_pipe = new Pipe(new PipeOptions(pauseWriterThreshold: pauseIncomingBytesThreshold, resumeWriterThreshold: resumeIncomingBytesThreshold));
_pipe = new Pipe();
_pipeReaderStream = _pipe.Reader.AsStream();
PipeReader = _pipe.Reader;
}
Expand Down
4 changes: 2 additions & 2 deletions src/Components/Server/src/Circuits/RemoteJSRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -157,8 +157,8 @@ public void MarkPermanentlyDisconnected()
_clientProxy = null;
}

protected override async Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, long pauseIncomingBytesThreshold = -1, long resumeIncomingBytesThreshold = -1, CancellationToken cancellationToken = default)
=> await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(this, jsStreamReference, totalLength, _maximumIncomingBytes, _options.JSInteropDefaultCallTimeout, pauseIncomingBytesThreshold, resumeIncomingBytesThreshold, cancellationToken);
protected override async Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, CancellationToken cancellationToken = default)
=> await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(this, jsStreamReference, totalLength, _maximumIncomingBytes, _options.JSInteropDefaultCallTimeout, cancellationToken);

public static class Log
{
Expand Down
16 changes: 6 additions & 10 deletions src/Components/Server/test/Circuits/RemoteJSDataStreamTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public async Task CreateRemoteJSDataStreamAsync_CreatesStream()
var jsStreamReference = Mock.Of<IJSStreamReference>();

// Act
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(_jsRuntime, jsStreamReference, totalLength: 100, maximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), pauseIncomingBytesThreshold: 50, resumeIncomingBytesThreshold: 25, cancellationToken: CancellationToken.None).DefaultTimeout();
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(_jsRuntime, jsStreamReference, totalLength: 100, signalRMaximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), cancellationToken: CancellationToken.None).DefaultTimeout();

// Assert
Assert.NotNull(remoteJSDataStream);
Expand Down Expand Up @@ -146,7 +146,7 @@ public async Task ReceiveData_ProvidedWithMoreBytesThanRemaining()
// Arrange
var jsRuntime = new TestRemoteJSRuntime(Options.Create(new CircuitOptions()), Options.Create(new HubOptions()), Mock.Of<ILogger<RemoteJSRuntime>>());
var jsStreamReference = Mock.Of<IJSStreamReference>();
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(jsRuntime, jsStreamReference, totalLength: 100, maximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), pauseIncomingBytesThreshold: 50, resumeIncomingBytesThreshold: 25, cancellationToken: CancellationToken.None);
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(jsRuntime, jsStreamReference, totalLength: 100, signalRMaximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), cancellationToken: CancellationToken.None);
var streamId = GetStreamId(remoteJSDataStream, jsRuntime);
var chunk = new byte[110]; // 100 byte totalLength for stream

Expand All @@ -166,7 +166,7 @@ public async Task ReceiveData_ProvidedWithOutOfOrderChunk_SimulatesSignalRDiscon
// Arrange
var jsRuntime = new TestRemoteJSRuntime(Options.Create(new CircuitOptions()), Options.Create(new HubOptions()), Mock.Of<ILogger<RemoteJSRuntime>>());
var jsStreamReference = Mock.Of<IJSStreamReference>();
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(jsRuntime, jsStreamReference, totalLength: 100, maximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), pauseIncomingBytesThreshold: 50, resumeIncomingBytesThreshold: 25, cancellationToken: CancellationToken.None);
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(jsRuntime, jsStreamReference, totalLength: 100, signalRMaximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), cancellationToken: CancellationToken.None);
var streamId = GetStreamId(remoteJSDataStream, jsRuntime);
var chunk = new byte[5];

Expand Down Expand Up @@ -201,10 +201,8 @@ public async Task ReceiveData_NoDataProvidedBeforeTimeout_StreamDisposed()
jsRuntime,
jsStreamReference,
totalLength: 15,
maximumIncomingBytes: 10_000,
signalRMaximumIncomingBytes: 10_000,
jsInteropDefaultCallTimeout: TimeSpan.FromSeconds(2),
pauseIncomingBytesThreshold: 50,
resumeIncomingBytesThreshold: 25,
cancellationToken: CancellationToken.None);
var streamId = GetStreamId(remoteJSDataStream, jsRuntime);
var chunk = new byte[] { 3, 5, 7 };
Expand Down Expand Up @@ -244,10 +242,8 @@ public async Task ReceiveData_ReceivesDataThenTimesout_StreamDisposed()
jsRuntime,
jsStreamReference,
totalLength: 15,
maximumIncomingBytes: 10_000,
signalRMaximumIncomingBytes: 10_000,
jsInteropDefaultCallTimeout: TimeSpan.FromSeconds(3),
pauseIncomingBytesThreshold: 50,
resumeIncomingBytesThreshold: 25,
cancellationToken: CancellationToken.None);
var streamId = GetStreamId(remoteJSDataStream, jsRuntime);
var chunk = new byte[] { 3, 5, 7 };
Expand Down Expand Up @@ -281,7 +277,7 @@ public async Task ReceiveData_ReceivesDataThenTimesout_StreamDisposed()
private static async Task<RemoteJSDataStream> CreateRemoteJSDataStreamAsync(TestRemoteJSRuntime jsRuntime = null)
{
var jsStreamReference = Mock.Of<IJSStreamReference>();
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(jsRuntime ?? _jsRuntime, jsStreamReference, totalLength: 100, maximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), pauseIncomingBytesThreshold: 50, resumeIncomingBytesThreshold: 25, cancellationToken: CancellationToken.None);
var remoteJSDataStream = await RemoteJSDataStream.CreateRemoteJSDataStreamAsync(jsRuntime ?? _jsRuntime, jsStreamReference, totalLength: 100, signalRMaximumIncomingBytes: 10_000, jsInteropDefaultCallTimeout: TimeSpan.FromMinutes(1), cancellationToken: CancellationToken.None);
return remoteJSDataStream;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public static void NotifyByteArrayAvailable(int id)
}

/// <inheritdoc />
protected override Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, long pauseIncomingBytesThreshold = -1, long resumeIncomingBytesThreshold = -1, CancellationToken cancellationToken = default)
protected override Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, CancellationToken cancellationToken = default)
=> Task.FromResult<Stream>(PullFromJSDataStream.CreateJSDataStream(this, jsStreamReference, totalLength, cancellationToken));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ protected override void SendByteArray(int id, byte[] data)
_ipcSender.SendByteArray(id, data);
}

protected override Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, long pauseIncomingBytesThreshold = -1, long resumeIncomingBytesThreshold = -1, CancellationToken cancellationToken = default)
protected override Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, CancellationToken cancellationToken = default)
=> Task.FromResult<Stream>(PullFromJSDataStream.CreateJSDataStream(this, jsStreamReference, totalLength, cancellationToken));
}
}
19 changes: 1 addition & 18 deletions src/JSInterop/Microsoft.JSInterop/src/IJSStreamReference.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,8 @@ public interface IJSStreamReference : IAsyncDisposable
/// Opens a <see cref="Stream"/> with the <see cref="JSRuntime"/> for the current data reference.
/// </summary>
/// <param name="maxAllowedSize">Maximum number of bytes permitted to be read from JavaScript.</param>
/// <param name="pauseIncomingBytesThreshold">
/// The number of unconsumed bytes to accept from JS before blocking.
/// Defaults to -1, which indicates use of the default <see cref="System.IO.Pipelines.PipeOptions.PauseWriterThreshold" />.
/// Avoid specifying an excessively large value because this could allow clients to exhaust memory.
/// A value of zero prevents JS from blocking, allowing .NET to receive an unlimited number of bytes.
/// <para>
/// This only has an effect when using Blazor Server.
/// </para>
/// </param>
/// <param name="resumeIncomingBytesThreshold">
/// The number of unflushed bytes at which point JS stops blocking.
/// Defaults to -1, which indicates use of the default <see cref="System.IO.Pipelines.PipeOptions.PauseWriterThreshold" />.
/// Must be less than the <paramref name="pauseIncomingBytesThreshold"/> to prevent thrashing at the limit.
/// <para>
/// This only has an effect when using Blazor Server.
/// </para>
/// </param>
/// <param name="cancellationToken"><see cref="CancellationToken" /> for cancelling read.</param>
/// <returns><see cref="Stream"/> which can provide data associated with the current data reference.</returns>
ValueTask<Stream> OpenReadStreamAsync(long maxAllowedSize = 512000, long pauseIncomingBytesThreshold = -1, long resumeIncomingBytesThreshold = -1, CancellationToken cancellationToken = default);
ValueTask<Stream> OpenReadStreamAsync(long maxAllowedSize = 512000, CancellationToken cancellationToken = default);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,14 @@ internal JSStreamReference(JSRuntime jsRuntime, long id, long totalLength) : bas
}

/// <inheritdoc />
async ValueTask<Stream> IJSStreamReference.OpenReadStreamAsync(long maxLength, long pauseIncomingBytesThreshold, long resumeIncomingBytesThreshold, CancellationToken cancellationToken)
async ValueTask<Stream> IJSStreamReference.OpenReadStreamAsync(long maxLength, CancellationToken cancellationToken)
{
if (Length > maxLength)
{
throw new ArgumentOutOfRangeException(nameof(maxLength), $"The incoming data stream of length {Length} exceeds the maximum length {maxLength}.");
}

return await _jsRuntime.ReadJSDataAsStreamAsync(this, Length, pauseIncomingBytesThreshold, resumeIncomingBytesThreshold, cancellationToken);
return await _jsRuntime.ReadJSDataAsStreamAsync(this, Length, cancellationToken);
}
}
}
19 changes: 1 addition & 18 deletions src/JSInterop/Microsoft.JSInterop/src/JSRuntime.cs
Original file line number Diff line number Diff line change
Expand Up @@ -215,26 +215,9 @@ protected internal virtual void ReceiveByteArray(int id, byte[] data)
/// </summary>
/// <param name="jsStreamReference"><see cref="IJSStreamReference"/> to produce a data stream for.</param>
/// <param name="totalLength">Expected length of the incoming data stream.</param>
/// <param name="pauseIncomingBytesThreshold">
/// The number of unconsumed bytes to accept from JS before blocking.
/// Defaults to -1, which indicates use of the default <see cref="System.IO.Pipelines.PipeOptions.PauseWriterThreshold" />.
/// Avoid specifying an excessively large value because this could allow clients to exhaust memory.
/// A value of zero prevents JS from blocking, allowing .NET to receive an unlimited number of bytes.
/// <para>
/// This only has an effect when using Blazor Server.
/// </para>
/// </param>
/// <param name="resumeIncomingBytesThreshold">
/// The number of unflushed bytes at which point JS stops blocking.
/// Defaults to -1, which indicates use of the default <see cref="System.IO.Pipelines.PipeOptions.PauseWriterThreshold" />.
/// Must be less than the <paramref name="pauseIncomingBytesThreshold"/> to prevent thrashing at the limit.
/// <para>
/// This only has an effect when using Blazor Server.
/// </para>
/// </param>
/// <param name="cancellationToken"><see cref="CancellationToken" /> for cancelling read.</param>
/// <returns><see cref="Stream"/> for the data reference represented by <paramref name="jsStreamReference"/>.</returns>
protected internal virtual Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, long pauseIncomingBytesThreshold = -1, long resumeIncomingBytesThreshold = -1, CancellationToken cancellationToken = default)
protected internal virtual Task<Stream> ReadJSDataAsStreamAsync(IJSStreamReference jsStreamReference, long totalLength, CancellationToken cancellationToken = default)
{
// The reason it's virtual and not abstract is just for back-compat

Expand Down
4 changes: 2 additions & 2 deletions src/JSInterop/Microsoft.JSInterop/src/PublicAPI.Unshipped.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#nullable enable
Microsoft.JSInterop.IJSStreamReference
Microsoft.JSInterop.IJSStreamReference.Length.get -> long
Microsoft.JSInterop.IJSStreamReference.OpenReadStreamAsync(long maxAllowedSize = 512000, long pauseIncomingBytesThreshold = -1, long resumeIncomingBytesThreshold = -1, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask<System.IO.Stream!>
Microsoft.JSInterop.IJSStreamReference.OpenReadStreamAsync(long maxAllowedSize = 512000, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.ValueTask<System.IO.Stream!>
Microsoft.JSInterop.Implementation.JSStreamReference
Microsoft.JSInterop.Implementation.JSStreamReference.Length.get -> long
Microsoft.JSInterop.Implementation.JSObjectReferenceJsonWorker
Expand Down Expand Up @@ -36,7 +36,7 @@ static Microsoft.JSInterop.JSRuntimeExtensions.InvokeVoidAsync(this Microsoft.JS
static Microsoft.JSInterop.JSRuntimeExtensions.InvokeVoidAsync(this Microsoft.JSInterop.IJSRuntime! jsRuntime, string! identifier, System.TimeSpan timeout, params object?[]? args) -> System.Threading.Tasks.ValueTask
*REMOVED*static Microsoft.JSInterop.JSRuntimeExtensions.InvokeVoidAsync(this Microsoft.JSInterop.IJSRuntime! jsRuntime, string! identifier, params object![]! args) -> System.Threading.Tasks.ValueTask
static Microsoft.JSInterop.JSRuntimeExtensions.InvokeVoidAsync(this Microsoft.JSInterop.IJSRuntime! jsRuntime, string! identifier, params object?[]? args) -> System.Threading.Tasks.ValueTask
virtual Microsoft.JSInterop.JSRuntime.ReadJSDataAsStreamAsync(Microsoft.JSInterop.IJSStreamReference! jsStreamReference, long totalLength, long pauseIncomingBytesThreshold = -1, long resumeIncomingBytesThreshold = -1, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<System.IO.Stream!>!
virtual Microsoft.JSInterop.JSRuntime.ReadJSDataAsStreamAsync(Microsoft.JSInterop.IJSStreamReference! jsStreamReference, long totalLength, System.Threading.CancellationToken cancellationToken = default(System.Threading.CancellationToken)) -> System.Threading.Tasks.Task<System.IO.Stream!>!
virtual Microsoft.JSInterop.JSRuntime.ReceiveByteArray(int id, byte[]! data) -> void
virtual Microsoft.JSInterop.JSRuntime.SendByteArray(int id, byte[]! data) -> void
Microsoft.JSInterop.JSDisconnectedException
Expand Down
2 changes: 1 addition & 1 deletion src/JSInterop/Microsoft.JSInterop/test/JSRuntimeTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,7 @@ public async void ReadJSDataAsStreamAsync_ThrowsNotSupportedException()
var dataReference = new JSStreamReference(runtime, 10, 10);

// Act
var exception = await Assert.ThrowsAsync<NotSupportedException>(async () => await runtime.ReadJSDataAsStreamAsync(dataReference, 10, 10, 10, CancellationToken.None));
var exception = await Assert.ThrowsAsync<NotSupportedException>(async () => await runtime.ReadJSDataAsStreamAsync(dataReference, 10, CancellationToken.None));

// Assert
Assert.Equal("The current JavaScript runtime does not support reading data streams.", exception.Message);
Expand Down