From 8ef285620ce7a8f07bc866295b0be39fc0aa5d8f Mon Sep 17 00:00:00 2001 From: Mikael Mengistu Date: Fri, 9 Aug 2019 19:04:33 -0700 Subject: [PATCH] Remove references to OnReaderCompleted and OnWriterCompleted (#13018) --- ...HttpConnectionTests.ConnectionLifecycle.cs | 3 +- .../HubConnectionTests.ConnectionLifecycle.cs | 6 +-- .../Client/test/UnitTests/TestConnection.cs | 10 +--- .../test/HttpConnectionManagerTests.cs | 39 ++++++---------- .../Tests.Utils/PipeCompletionExtensions.cs | 46 ------------------- 5 files changed, 18 insertions(+), 86 deletions(-) delete mode 100644 src/SignalR/common/testassets/Tests.Utils/PipeCompletionExtensions.cs diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.ConnectionLifecycle.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.ConnectionLifecycle.cs index 1c2280e42d..fa95fbc83b 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.ConnectionLifecycle.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HttpConnectionTests.ConnectionLifecycle.cs @@ -316,8 +316,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests await connection.StartAsync().OrTimeout(); await connection.Transport.Output.WriteAsync(new byte[] { 0x42 }).OrTimeout(); - // We should get the exception in the transport input completion. - await Assert.ThrowsAsync(() => connection.Transport.Input.WaitForWriterToComplete()).OrTimeout(); + await Assert.ThrowsAsync(async () => await connection.Transport.Input.ReadAsync()); }); } } diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.ConnectionLifecycle.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.ConnectionLifecycle.cs index 02a5ae1961..3c669ef94d 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.ConnectionLifecycle.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/HubConnectionTests.ConnectionLifecycle.cs @@ -367,10 +367,6 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests var connectionClosed = new TaskCompletionSource(); await AsyncUsing(CreateHubConnection(testConnection), async connection => { -#pragma warning disable 0618 - // We're hooking the TestConnection shutting down here because the HubConnection one will be blocked on the lock - testConnection.Transport.Input.OnWriterCompleted((_, __) => testConnectionClosed.TrySetResult(null), null); -#pragma warning restore connection.Closed += (e) => { connectionClosed.TrySetResult(null); @@ -385,7 +381,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests testConnection.CompleteFromTransport(); // Wait for the connection to close. - await testConnectionClosed.Task.OrTimeout(); + await testConnection.Transport.Input.CompleteAsync(); // The stop should be completed. await stopTask.OrTimeout(); diff --git a/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs b/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs index 806863e3f5..fbc516e95c 100644 --- a/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs +++ b/src/SignalR/clients/csharp/Client/test/UnitTests/TestConnection.cs @@ -52,15 +52,6 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests var pair = DuplexPipe.CreateConnectionPair(options, options); Application = pair.Application; Transport = pair.Transport; - - // TODO: Resolve this, for now we use Pipe which works -#pragma warning disable 0618 - Application.Input.OnWriterCompleted((ex, _) => - { - Application.Output.Complete(); - }, - null); -#pragma warning restore 0618 } public override ValueTask DisposeAsync() => DisposeCoreAsync(); @@ -156,6 +147,7 @@ namespace Microsoft.AspNetCore.SignalR.Client.Tests } else if (result.IsCompleted) { + await Application.Output.CompleteAsync(); return null; } } diff --git a/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs b/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs index eae60a076d..5c30a490f7 100644 --- a/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs +++ b/src/SignalR/common/Http.Connections/test/HttpConnectionManagerTests.cs @@ -1,8 +1,6 @@ // Copyright (c) .NET Foundation. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -#pragma warning disable CS0618 // TODO: Remove when we replace the events - using System; using System.Buffers; using System.IO.Pipelines; @@ -100,11 +98,6 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests var transportInputTcs = new TaskCompletionSource(); var transportOutputTcs = new TaskCompletionSource(); - connection.Transport.Input.OnWriterCompleted((_, __) => transportInputTcs.TrySetResult(null), null); - connection.Transport.Output.OnReaderCompleted((_, __) => transportOutputTcs.TrySetResult(null), null); - connection.Application.Input.OnWriterCompleted((_, __) => applicationInputTcs.TrySetResult(null), null); - connection.Application.Output.OnReaderCompleted((_, __) => applicationOutputTcs.TrySetResult(null), null); - try { await connection.DisposeAsync(closeGracefully).OrTimeout(); @@ -114,7 +107,17 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests // Ignore the exception that bubbles out of the failing task } - await Task.WhenAll(applicationInputTcs.Task, applicationOutputTcs.Task, transportInputTcs.Task, transportOutputTcs.Task).OrTimeout(); + var result = await connection.Transport.Output.FlushAsync(); + Assert.True(result.IsCompleted); + + result = await connection.Application.Output.FlushAsync(); + Assert.True(result.IsCompleted); + + var exception = await Assert.ThrowsAsync(async () => await connection.Transport.Input.ReadAsync()); + Assert.Equal("Reading is not allowed after reader was completed.", exception.Message); + + exception = await Assert.ThrowsAsync(async () => await connection.Application.Input.ReadAsync()); + Assert.Equal("Reading is not allowed after reader was completed.", exception.Message); } } @@ -342,16 +345,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests var connection = connectionManager.CreateConnection(PipeOptions.Default, PipeOptions.Default); - connection.Application.Output.OnReaderCompleted((error, state) => - { - tcs.TrySetResult(null); - }, - null); - appLifetime.StopApplication(); - // Connection should be disposed so this should complete immediately - await tcs.Task.OrTimeout(); + var result = await connection.Application.Output.FlushAsync(); + Assert.True(result.IsCompleted); } } @@ -368,16 +365,10 @@ namespace Microsoft.AspNetCore.Http.Connections.Tests var connection = connectionManager.CreateConnection(PipeOptions.Default, PipeOptions.Default); - connection.Application.Output.OnReaderCompleted((error, state) => - { - tcs.TrySetResult(null); - }, - null); - appLifetime.StopApplication(); - // Connection should be disposed so this should complete immediately - await tcs.Task.OrTimeout(); + var result = await connection.Application.Output.FlushAsync(); + Assert.True(result.IsCompleted); } } diff --git a/src/SignalR/common/testassets/Tests.Utils/PipeCompletionExtensions.cs b/src/SignalR/common/testassets/Tests.Utils/PipeCompletionExtensions.cs deleted file mode 100644 index 03a45ea3fe..0000000000 --- a/src/SignalR/common/testassets/Tests.Utils/PipeCompletionExtensions.cs +++ /dev/null @@ -1,46 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -#pragma warning disable 0618 // TODO: Remove dependency on pipe events - -using System.Threading.Tasks; - -namespace System.IO.Pipelines -{ - public static class PipeCompletionExtensions - { - public static Task WaitForWriterToComplete(this PipeReader reader) - { - var tcs = new TaskCompletionSource(); - reader.OnWriterCompleted((ex, state) => - { - if (ex != null) - { - ((TaskCompletionSource)state).TrySetException(ex); - } - else - { - ((TaskCompletionSource)state).TrySetResult(null); - } - }, tcs); - return tcs.Task; - } - - public static Task WaitForReaderToComplete(this PipeWriter writer) - { - var tcs = new TaskCompletionSource(); - writer.OnReaderCompleted((ex, state) => - { - if (ex != null) - { - ((TaskCompletionSource)state).TrySetException(ex); - } - else - { - ((TaskCompletionSource)state).TrySetResult(null); - } - }, tcs); - return tcs.Task; - } - } -}