Merge pull request #108 from neutmute/issue-106b

Issue 106 - trap exceptions and make available via OnOperationCompleted event

Neo4jClient.Tests/GraphClientTests/ConnectTests.cs

@@ -245,5 +245,30 @@ public void ShouldFormatAuthorisationHeaderCorrectly()
 
             Assert.AreEqual(expectedHeader, httpClient.AuthenticationHeaderValue.Parameter);
         }
+
+        [Test]
+        public void ShouldFireOnCompletedEvenWhenException()
+        {
+            var httpClient = Substitute.For<IHttpClient>();
+            httpClient
+                .SendAsync(Arg.Any<HttpRequestMessage>())
+                .Returns(callInfo => { throw new NotImplementedException(); });
+
+            var graphClient = new GraphClient(new Uri("http://foo/db/data"), httpClient);
+            OperationCompletedEventArgs operationCompletedArgs = null;
+
+            graphClient.OperationCompleted+= (s, e) =>
+            {
+                operationCompletedArgs = e;
+            };
+
+            // act
+            Assert.Throws<NotImplementedException>(() => graphClient.Connect());
+
+            Assert.NotNull(operationCompletedArgs);
+            Assert.That(operationCompletedArgs.HasException);
+            Assert.AreEqual(typeof(NotImplementedException), operationCompletedArgs.Exception.GetType());
+        }
+
     }
 }

Neo4jClient.Tests/GraphClientTests/Cypher/ExecuteCypherTests.cs

@@ -71,6 +71,10 @@ public void ShouldSendCommandAndNotCareAboutResultsAsync()
             }
         }
 
+        /// <summary>
+        /// This predates #106. Given Tatham's guidance that the event should fire irrespective is this test proving correct behaviour?
+        /// In any case the sync method calls async so it might be hard to avoid double firing an event.
+        /// </summary>
         [Test]
         public void WhenAsyncCommandFails_ShouldNotRaiseCompleted()
         {
@@ -107,5 +111,85 @@ public void WhenAsyncCommandFails_ShouldNotRaiseCompleted()
                 Assert.IsFalse(raisedEvent, "Raised OperationCompleted");
             }
         }
+
+        /// <summary>
+        /// #106
+        /// </summary>
+        [Test]
+        public void WhenExecuteGetCypherResultsFails_ShouldRaiseCompletedWithException()
+        {
+            // Arrange
+            const string queryText = @"return 1";
+            var parameters = new Dictionary<string, object>();
+
+            var cypherQuery = new CypherQuery(queryText, parameters, CypherResultMode.Set);
+            var cypherApiQuery = new CypherApiQuery(cypherQuery);
+
+            using (var testHarness = new RestTestHarness
+            {
+                {
+                    MockRequest.PostObjectAsJson("/cypher", cypherApiQuery),
+                    MockResponse.Throws()
+                }
+            })
+            {
+                var graphClient = testHarness.CreateAndConnectGraphClient();
+
+                OperationCompletedEventArgs eventArgs = null;
+
+                graphClient.OperationCompleted += (sender, e) => { eventArgs = e; };
+
+                //Act
+                Assert.Throws<MockResponseThrowsException>(() =>
+                {
+                    graphClient.ExecuteGetCypherResults<ExecuteGetCypherResultsTests.SimpleResultDto>(cypherQuery);
+                }, "We should expect an exception");
+
+                Assert.IsNotNull(eventArgs, "but we should also have received the completion event");
+                Assert.IsTrue(eventArgs.HasException);
+                Assert.AreEqual(typeof(MockResponseThrowsException), eventArgs.Exception.GetType());
+                Assert.AreEqual(-1, eventArgs.ResourcesReturned);
+            }
+        }
+
+        /// <summary>
+        /// #106
+        /// </summary>
+        [Test]
+        public void WhenExecuteCypherFails_ShouldRaiseCompletedWithException()
+        {
+            // Arrange
+            const string queryText = @"bad cypher";
+            var parameters = new Dictionary<string, object>();
+
+            var cypherQuery = new CypherQuery(queryText, parameters, CypherResultMode.Set);
+            var cypherApiQuery = new CypherApiQuery(cypherQuery);
+
+            using (var testHarness = new RestTestHarness
+            {
+                {
+                    MockRequest.PostObjectAsJson("/cypher", cypherApiQuery),
+                    MockResponse.Throws()
+                }
+            })
+            {
+                var graphClient = testHarness.CreateAndConnectGraphClient();
+
+                OperationCompletedEventArgs eventArgs = null;
+
+                graphClient.OperationCompleted += (sender, e) => { eventArgs = e; };
+
+                //Act
+                Assert.Throws<MockResponseThrowsException>(() =>
+                {
+                    graphClient.ExecuteCypher(cypherQuery);
+                }, "We should expect an exception");
+
+                Assert.IsNotNull(eventArgs, "but we should also have received the completion event");
+                Assert.IsTrue(eventArgs.HasException);
+                Assert.AreEqual(typeof(MockResponseThrowsException), eventArgs.Exception.GetType());
+                Assert.AreEqual(-1, eventArgs.ResourcesReturned);
+            }
+        }
     }
 }

Neo4jClient.Tests/Transactions/QueriesInTransactionTests.cs

@@ -920,9 +920,14 @@ public void AsyncRequestsInTransactionShouldBeExecutedInOrder()
             }
         }
 
-
+        /// <summary>
+        /// This test is flakey. If run in Resharper as a group, it fails. If run by itself it passes.
+        /// Appears to be a race condition where for the test to pass the ExecuteGetCypherResultsAsync() call needs to still be in progress before the Commit() is called.
+        /// If stepped through, the test will fail since the call easily finishes. Flakeyness observable as early as [bdc1c45]
+        /// Perhaps need to insert simulated delay into MockResponse?
+        /// </summary>
         [Test]
-        [ExpectedException(typeof(InvalidOperationException))]
+        [ExpectedException(typeof(InvalidOperationException), ExpectedMessage = "Cannot commit unless all tasks have been completed")]
         public void CommitFailsOnPendingAsyncRequests()
         {
             const string queryText = @"MATCH (n) RETURN count(n) as Total";
@@ -948,8 +953,6 @@ public void CommitFailsOnPendingAsyncRequests()
                 }
 
             }
-
-            Assert.Fail("Commit did not fail with pending tasks");
         }
     }
 }