Merge pull request #6393 from jtattermusch/csharp_internal_cleanup

C# cleanup client-side call RECV_STATUS_ON_CLIENT internals

2 files changed, 107 insertions, 15 deletions

diff --git a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs
index 60530d3250..a678e4dafe 100644
--- a/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs
+++ b/src/csharp/Grpc.Core.Tests/Internal/AsyncCallTest.cs
@@ -64,28 +64,115 @@ namespace Grpc.Core.Internal.Tests
         }
 
         [Test]
-        public void AsyncUnary_CompletionSuccess()
+        public void AsyncUnary_CanBeStartedOnlyOnce()
+        {
+            asyncCall.UnaryCallAsync("request1");
+            Assert.Throws(typeof(InvalidOperationException),
+                () => asyncCall.UnaryCallAsync("abc"));
+        }
+
+        [Test]
+        public void AsyncUnary_StreamingOperationsNotAllowed()
+        {
+            asyncCall.UnaryCallAsync("request1");
+            Assert.Throws(typeof(InvalidOperationException),
+                () => asyncCall.StartReadMessage((x,y) => {}));
+            Assert.Throws(typeof(InvalidOperationException),
+                () => asyncCall.StartSendMessage("abc", new WriteFlags(), (x,y) => {}));
+        }
+
+        [Test]
+        public void AsyncUnary_Success()
+        {
+            var resultTask = asyncCall.UnaryCallAsync("request1");
+            fakeCall.UnaryResponseClientHandler(true,
+                new ClientSideStatus(Status.DefaultSuccess, new Metadata()),
+                CreateResponsePayload(),
+                new Metadata());
+
+            AssertUnaryResponseSuccess(asyncCall, fakeCall, resultTask);
+        }
+
+        [Test]
+        public void AsyncUnary_NonSuccessStatusCode()
+        {
+            var resultTask = asyncCall.UnaryCallAsync("request1");
+            fakeCall.UnaryResponseClientHandler(true,
+                CreateClientSideStatus(StatusCode.InvalidArgument),
+                CreateResponsePayload(),
+                new Metadata());
+
+            AssertUnaryResponseError(asyncCall, fakeCall, resultTask, StatusCode.InvalidArgument);
+        }
+
+        [Test]
+        public void AsyncUnary_NullResponsePayload()
+        {
+            var resultTask = asyncCall.UnaryCallAsync("request1");
+            fakeCall.UnaryResponseClientHandler(true,
+                new ClientSideStatus(Status.DefaultSuccess, new Metadata()),
+                null,
+                new Metadata());
+
+            // failure to deserialize will result in InvalidArgument status.
+            AssertUnaryResponseError(asyncCall, fakeCall, resultTask, StatusCode.Internal);
+        }
+
+        [Test]
+        public void ClientStreaming_NoRequest_Success()
+        {
+            var resultTask = asyncCall.ClientStreamingCallAsync();
+            fakeCall.UnaryResponseClientHandler(true,
+                new ClientSideStatus(Status.DefaultSuccess, new Metadata()),
+                CreateResponsePayload(),
+                new Metadata());
+
+            AssertUnaryResponseSuccess(asyncCall, fakeCall, resultTask);
+        }
+
+        [Test]
+        public void ClientStreaming_NoRequest_NonSuccessStatusCode()
+        {
+            var resultTask = asyncCall.ClientStreamingCallAsync();
+            fakeCall.UnaryResponseClientHandler(true,
+                CreateClientSideStatus(StatusCode.InvalidArgument),
+                CreateResponsePayload(),
+                new Metadata());
+
+            AssertUnaryResponseError(asyncCall, fakeCall, resultTask, StatusCode.InvalidArgument);
+        }
+
+        ClientSideStatus CreateClientSideStatus(StatusCode statusCode)
+        {
+            return new ClientSideStatus(new Status(statusCode, ""), new Metadata());
+        }
+
+        byte[] CreateResponsePayload()
+        {
+            return Marshallers.StringMarshaller.Serializer("response1");
+        }
+
+        static void AssertUnaryResponseSuccess(AsyncCall<string, string> asyncCall, FakeNativeCall fakeCall, Task<string> resultTask)
         {
-            var resultTask = asyncCall.UnaryCallAsync("abc");
-            fakeCall.UnaryResponseClientHandler(true, new ClientSideStatus(Status.DefaultSuccess, new Metadata()), new byte[] { 1, 2, 3 }, new Metadata());
             Assert.IsTrue(resultTask.IsCompleted);
             Assert.IsTrue(fakeCall.IsDisposed);
+
             Assert.AreEqual(Status.DefaultSuccess, asyncCall.GetStatus());
+            Assert.AreEqual(0, asyncCall.ResponseHeadersAsync.Result.Count);
+            Assert.AreEqual(0, asyncCall.GetTrailers().Count);
+            Assert.AreEqual("response1", resultTask.Result);
         }
 
-        [Test]
-        public void AsyncUnary_CompletionFailure()
+        static void AssertUnaryResponseError(AsyncCall<string, string> asyncCall, FakeNativeCall fakeCall, Task<string> resultTask, StatusCode expectedStatusCode)
         {
-            var resultTask = asyncCall.UnaryCallAsync("abc");
-            fakeCall.UnaryResponseClientHandler(false, new ClientSideStatus(new Status(StatusCode.Internal, ""), null), new byte[] { 1, 2, 3 }, new Metadata());
-
             Assert.IsTrue(resultTask.IsCompleted);
             Assert.IsTrue(fakeCall.IsDisposed);
 
-            Assert.AreEqual(StatusCode.Internal, asyncCall.GetStatus().StatusCode);
-            Assert.IsNull(asyncCall.GetTrailers());
+            Assert.AreEqual(expectedStatusCode, asyncCall.GetStatus().StatusCode);
             var ex = Assert.ThrowsAsync<RpcException>(async () => await resultTask);
-            Assert.AreEqual(StatusCode.Internal, ex.Status.StatusCode);
+            Assert.AreEqual(expectedStatusCode, ex.Status.StatusCode);
+            Assert.AreEqual(0, asyncCall.ResponseHeadersAsync.Result.Count);
+            Assert.AreEqual(0, asyncCall.GetTrailers().Count);
         }
 
         internal class FakeNativeCall : INativeCall
diff --git a/src/csharp/Grpc.Core/Internal/AsyncCall.cs b/src/csharp/Grpc.Core/Internal/AsyncCall.cs
index 016e1b8587..50ba617cdb 100644
--- a/src/csharp/Grpc.Core/Internal/AsyncCall.cs
+++ b/src/csharp/Grpc.Core/Internal/AsyncCall.cs
@@ -409,10 +409,13 @@ namespace Grpc.Core.Internal
         /// </summary>
         private void HandleUnaryResponse(bool success, ClientSideStatus receivedStatus, byte[] receivedMessage, Metadata responseHeaders)
         {
+            // NOTE: because this event is a result of batch containing GRPC_OP_RECV_STATUS_ON_CLIENT,
+            // success will be always set to true.
+
             using (Profilers.ForCurrentThread().NewScope("AsyncCall.HandleUnaryResponse"))
             {
                 TResponse msg = default(TResponse);
-                var deserializeException = success ? TryDeserialize(receivedMessage, out msg) : null;
+                var deserializeException = TryDeserialize(receivedMessage, out msg);
 
                 lock (myLock)
                 {
@@ -425,14 +428,13 @@ namespace Grpc.Core.Internal
                     finishedStatus = receivedStatus;
 
                     ReleaseResourcesIfPossible();
-
                 }
 
                 responseHeadersTcs.SetResult(responseHeaders);
 
                 var status = receivedStatus.Status;
 
-                if (!success || status.StatusCode != StatusCode.OK)
+                if (status.StatusCode != StatusCode.OK)
                 {
                     unaryResponseTcs.SetException(new RpcException(status));
                     return;
@@ -447,6 +449,9 @@ namespace Grpc.Core.Internal
         /// </summary>
         private void HandleFinished(bool success, ClientSideStatus receivedStatus)
         {
+            // NOTE: because this event is a result of batch containing GRPC_OP_RECV_STATUS_ON_CLIENT,
+            // success will be always set to true.
+
             lock (myLock)
             {
                 finished = true;
@@ -457,7 +462,7 @@ namespace Grpc.Core.Internal
 
             var status = receivedStatus.Status;
 
-            if (!success || status.StatusCode != StatusCode.OK)
+            if (status.StatusCode != StatusCode.OK)
             {
                 streamingCallFinishedTcs.SetException(new RpcException(status));
                 return;