STU3: Do not dispose of a shared HttpMessageHandler passed into the constructor

src/Hl7.Fhir.Core.Tests/Rest/FhirClientTests.cs

@@ -1,7 +1,7 @@
-/* 
+/*
  * Copyright (c) 2014, Firely (info@fire.ly) and contributors
  * See the file CONTRIBUTORS for details.
- * 
+ *
  * This file is licensed under the BSD 3-Clause license
  * available at https://raw.githubusercontent.com/FirelyTeam/firely-net-sdk/master/LICENSE
  */
@@ -40,11 +40,12 @@ public class FhirClientTests
         //public static Uri testEndpoint = new Uri("http://sqlonfhir-stu3.azurewebsites.net/fhir");
         public static Uri testEndpoint = new Uri("https://server.fire.ly/r3");
 
-        //public static Uri _endpointSupportingSearchUsingPost = new Uri("http://localhost:49911/fhir"); 
+        //public static Uri _endpointSupportingSearchUsingPost = new Uri("http://localhost:49911/fhir");
         public static Uri _endpointSupportingSearchUsingPost = new Uri("http://localhost:4080");
         //public static Uri _endpointSupportingSearchUsingPost = new Uri("https://vonk.fire.ly/r3");
 
         public static Uri TerminologyEndpoint = new Uri("https://stu3.ontoserver.csiro.au/fhir");
+        // public static Uri TerminologyEndpoint = new Uri("http://test.fhir.org/r3");
 
         private static string patientId = "pat1" + ModelInfo.Version;
         private static string locationId = "loc1" + ModelInfo.Version;
@@ -315,7 +316,7 @@ public async T.Task ReadHttpClient()
                 await testReadClientAsync(client);
             }
         }
-        
+
         private async T.Task testReadClientAsync(BaseFhirClient client)
         {
             var loc = client.Read<Location>("Location/" + locationId);
@@ -387,7 +388,7 @@ private void testReadRelative(BaseFhirClient client)
 		public void ReadRelativeAsync()
 		{
 			FhirClient client = new FhirClient(testEndpoint);
-            testRelativeAsyncClient(client);			
+            testRelativeAsyncClient(client);
 		}
 
         [TestMethod, TestCategory("FhirClient")]
@@ -611,6 +612,44 @@ private void testSearchAsyncHttpClient(BaseFhirClient client)
             Assert.IsNotNull(result);
             Assert.IsTrue(result.Entry.Count > 0);
         }
+
+        public void SearchAsyncHttpClient()
+        {
+            using(TestClient client = new TestClient(testEndpoint))
+            {
+                Bundle result;
+
+                result = client.SearchAsync<DiagnosticReport>().Result;
+                Assert.IsNotNull(result);
+                Assert.IsTrue(result.Entry.Count() > 10, "Test should use testdata with more than 10 reports");
+
+                result = client.SearchAsync<DiagnosticReport>(pageSize: 10).Result;
+                Assert.IsNotNull(result);
+                Assert.IsTrue(result.Entry.Count <= 10);
+
+                var withSubject =
+                    result.Entry.ByResourceType<DiagnosticReport>().FirstOrDefault(dr => dr.Resource.Subject != null);
+                Assert.IsNotNull(withSubject, "Test should use testdata with a report with a subject");
+
+                ResourceIdentity ri = new ResourceIdentity(withSubject.Id);
+
+                result = client.SearchByIdAsync<DiagnosticReport>(ri.Id,
+                            includes: new string[] { "DiagnosticReport.subject" }).Result;
+                Assert.IsNotNull(result);
+
+                Assert.AreEqual(2, result.Entry.Count);  // should have subject too
+
+                Assert.IsNotNull(result.Entry.Single(entry => new ResourceIdentity(entry.Id).Collection ==
+                            typeof(DiagnosticReport).GetCollectionName()));
+                Assert.IsNotNull(result.Entry.Single(entry => new ResourceIdentity(entry.Id).Collection ==
+                            typeof(Patient).GetCollectionName()));
+
+                result = client.SearchAsync<Patient>(new string[] { "name=Everywoman", "name=Eve" }).Result;
+
+                Assert.IsNotNull(result);
+                Assert.IsTrue(result.Entry.Count > 0);
+            }
+        }
 #endif
 
 
@@ -912,7 +951,7 @@ private static void testCreateEditDeleteAsync(BaseFhirClient client)
                 Name = "Furore",
                 Identifier = new List<Identifier> { new Identifier("http://hl7.org/test/1", "3141") },
                 Telecom = new List<ContactPoint> { new ContactPoint { System = ContactPoint.ContactPointSystem.Phone, Value = "+31-20-3467171" } }
-            };          
+            };
 
             var fe = client.CreateAsync<Organization>(furore).Result;
 
@@ -926,9 +965,9 @@ private static void testCreateEditDeleteAsync(BaseFhirClient client)
             var fe2 = client.UpdateAsync(fe).Result;
 
             Assert.IsNotNull(fe2);
-            Assert.AreEqual(fe.Id, fe2.Id);        
+            Assert.AreEqual(fe.Id, fe2.Id);
+
 
-
 
             fe.Identifier.Add(new Identifier("http://hl7.org/test/3", "3141592"));
             var fe3 = client.UpdateAsync(fe2).Result;
@@ -952,7 +991,7 @@ private static void testCreateEditDeleteAsync(BaseFhirClient client)
 
 
         /// <summary>
-        /// This test will fail if the system records AuditEvents 
+        /// This test will fail if the system records AuditEvents
         /// and counts them in the WholeSystemHistory
         /// </summary>
         [TestMethod, TestCategory("FhirClient"), TestCategory("IntegrationTest"), Ignore]     // Keeps on failing periodically. Grahames server?
@@ -964,7 +1003,7 @@ public async T.Task History()
 
 
         /// <summary>
-        /// This test will fail if the system records AuditEvents 
+        /// This test will fail if the system records AuditEvents
         /// and counts them in the WholeSystemHistory
         /// </summary>
         [TestMethod, TestCategory("FhirClient"), TestCategory("IntegrationTest"), Ignore]     // Keeps on failing periodically. Grahames server?
@@ -998,7 +1037,7 @@ await testCreateEditDeleteAsync(client); // this test does a create, update, upd
             history = client.TypeHistory("Patient", timestampBeforeCreationAndDeletions.ToUniversalTime());
             Assert.IsNotNull(history);
             DebugDumpBundle(history);
-            Assert.AreEqual(4, history.Entry.Count());   // there's a race condition here, sometimes this is 5. 
+            Assert.AreEqual(4, history.Entry.Count());   // there's a race condition here, sometimes this is 5.
             Assert.AreEqual(3, history.Entry.Where(entry => entry.Resource != null).Count());
             Assert.AreEqual(1, history.Entry.Where(entry => entry.IsDeleted()).Count());
 
@@ -1184,8 +1223,8 @@ private void verifyMeta(Meta meta, bool hasNew, int key)
         [TestCategory("FhirClient"), TestCategory("IntegrationTest")]
         public void TestSearchUsingPostMultipleIncludesShouldNotThrowArgumentException()
         {
-            // This test case proves issue https://github.com/FirelyTeam/firely-net-sdk/issues/1206 is fixed. 
-            // Previoulsly EntryToHttpExtensions.setBodyAndContentType used a Dictionary which required the 
+            // This test case proves issue https://github.com/FirelyTeam/firely-net-sdk/issues/1206 is fixed.
+            // Previoulsly EntryToHttpExtensions.setBodyAndContentType used a Dictionary which required the
             // name part of the parameters to be unique.
             // Fixed by using IEnumerable<KeyValuePair<string, string>> instead of Dictionary<string, string>
             var client = new LegacyFhirClient(testEndpoint);
@@ -1197,8 +1236,8 @@ public void TestSearchUsingPostMultipleIncludesShouldNotThrowArgumentException()
         [TestCategory("FhirClient"), TestCategory("IntegrationTest")]
         public void TestSearchUsingPostMultipleIncludesShouldNotThrowArgumentExceptionHttpClient()
         {
-            // This test case proves issue https://github.com/FirelyTeam/firely-net-sdk/issues/1206 is fixed. 
-            // Previoulsly EntryToHttpExtensions.setBodyAndContentType used a Dictionary which required the 
+            // This test case proves issue https://github.com/FirelyTeam/firely-net-sdk/issues/1206 is fixed.
+            // Previoulsly EntryToHttpExtensions.setBodyAndContentType used a Dictionary which required the
             // name part of the parameters to be unique.
             // Fixed by using IEnumerable<KeyValuePair<string, string>> instead of Dictionary<string, string>
             var client = new FhirClient(testEndpoint);
@@ -1347,43 +1386,85 @@ public void CallsCallbacks()
         public void CallsCallbacksHttpClient()
         {
             using (var handler = new HttpClientEventHandler())
-            using (FhirClient client = new FhirClient(testEndpoint, messageHandler: handler))
             {
-                client.Settings.ParserSettings.AllowUnrecognizedEnums = true;
+                using (FhirClient client = new FhirClient(testEndpoint, messageHandler: handler))
+                {
+                    client.Settings.ParserSettings.AllowUnrecognizedEnums = true;
 
-                bool calledBefore = false;
-                HttpStatusCode? status = null;
-                byte[] body = null;
-                byte[] bodyOut = null;
+                    bool calledBefore = false;
+                    HttpStatusCode? status = null;
+                    byte[] body = null;
+                    byte[] bodyOut = null;
 
-                handler.OnBeforeRequest += (sender, e) =>
-                {
-                    calledBefore = true;
-                    bodyOut = e.Body;
-                };
+                    handler.OnBeforeRequest += (sender, e) =>
+                    {
+                        calledBefore = true;
+                        bodyOut = e.Body;
+                    };
+
+                    handler.OnAfterResponse += (sender, e) =>
+                    {
+                        body = e.Body;
+                        status = e.RawResponse.StatusCode;
+                    };
+
+                    var pat = client.Read<Patient>("Patient/" + patientId);
+                    Assert.IsTrue(calledBefore);
+                    Assert.IsNotNull(status);
+                    Assert.IsNotNull(body);
 
-                handler.OnAfterResponse += (sender, e) =>
+                    var bodyText = HttpUtil.DecodeBody(body, Encoding.UTF8);
+
+                    Assert.IsTrue(bodyText.Contains("<Patient"));
+
+                    calledBefore = false;
+                    client.Update(pat); // create cannot be called with an ID (which was retrieved)
+                    Assert.IsTrue(calledBefore);
+                    Assert.IsNotNull(bodyOut);
+
+                    bodyText = HttpUtil.DecodeBody(body, Encoding.UTF8);
+                    Assert.IsTrue(bodyText.Contains("<Patient"));
+                }
+
+                // And use another on the same handler to ensure that it wasn't disposed :O
+                using (FhirClient client = new FhirClient(testEndpoint, messageHandler: handler))
                 {
-                    body = e.Body;
-                    status = e.RawResponse.StatusCode;
-                };
+                    client.Settings.ParserSettings.AllowUnrecognizedEnums = true;
 
-                var pat = client.Read<Patient>("Patient/" + patientId);
-                Assert.IsTrue(calledBefore);
-                Assert.IsNotNull(status);
-                Assert.IsNotNull(body);
+                    bool calledBefore = false;
+                    HttpStatusCode? status = null;
+                    byte[] body = null;
+                    byte[] bodyOut = null;
 
-                var bodyText = HttpUtil.DecodeBody(body, Encoding.UTF8);
+                    handler.OnBeforeRequest += (sender, e) =>
+                    {
+                        calledBefore = true;
+                        bodyOut = e.Body;
+                    };
 
-                Assert.IsTrue(bodyText.Contains("<Patient"));
+                    handler.OnAfterResponse += (sender, e) =>
+                    {
+                        body = e.Body;
+                        status = e.RawResponse.StatusCode;
+                    };
+
+                    var pat = client.Read<Patient>("Patient/" + patientId);
+                    Assert.IsTrue(calledBefore);
+                    Assert.IsNotNull(status);
+                    Assert.IsNotNull(body);
+
+                    var bodyText = HttpUtil.DecodeBody(body, Encoding.UTF8);
 
-                calledBefore = false;
-                client.Update(pat); // create cannot be called with an ID (which was retrieved)
-                Assert.IsTrue(calledBefore);
-                Assert.IsNotNull(bodyOut);
+                    Assert.IsTrue(bodyText.Contains("<Patient"));
 
-                bodyText = HttpUtil.DecodeBody(body, Encoding.UTF8);
-                Assert.IsTrue(bodyText.Contains("<Patient"));
+                    calledBefore = false;
+                    client.Update(pat); // create cannot be called with an ID (which was retrieved)
+                    Assert.IsTrue(calledBefore);
+                    Assert.IsNotNull(bodyOut);
+
+                    bodyText = HttpUtil.DecodeBody(body, Encoding.UTF8);
+                    Assert.IsTrue(bodyText.Contains("<Patient"));
+                }
             }
         }
 

src/Hl7.Fhir.Core/Rest/FhirClient.cs

@@ -1,7 +1,7 @@
-/* 
+/*
 * Copyright (c) 2014, Firely (info@fire.ly) and contributors
 * See the file CONTRIBUTORS for details.
-* 
+*
 * This file is licensed under the BSD 3-Clause license
 * available at https://raw.githubusercontent.com/FirelyTeam/firely-net-sdk/master/LICENSE
 */
@@ -21,11 +21,14 @@ public partial class FhirClient : BaseFhirClient
     {
 //disables warning that OnBeforeRequest and OnAfterResponse are never used.
 #pragma warning disable CS0067
-        
+
         /// <summary>
         /// Creates a new client using a default endpoint
         /// If the endpoint does not end with a slash (/), it will be added.
         /// </summary>
+        /// <remarks>
+        /// If the messageHandler is provided then it must be disposed by the caller
+        /// </remarks>
         /// <param name="endpoint">
         /// The URL of the server to connect to.<br/>
         /// If the trailing '/' is not present, then it will be appended automatically
@@ -41,7 +44,7 @@ public FhirClient(Uri endpoint, FhirClientSettings settings = null, HttpMessageH
                 AutomaticDecompression = DecompressionMethods.GZip | DecompressionMethods.Deflate
             };
 
-            var requester = new HttpClientRequester(Endpoint, Settings, handler);
+            var requester = new HttpClientRequester(Endpoint, Settings, handler, messageHandler == null);
             Requester = requester;
 
             // Expose default request headers to user.
@@ -122,7 +125,7 @@ public bool ReturnFullResource
         }
 
         /// <summary>
-        /// Should calls to Create, Update and transaction operations return the whole updated content, 
+        /// Should calls to Create, Update and transaction operations return the whole updated content,
         /// or an OperationOutcome?
         /// </summary>
         /// <remarks>Refer to specification section 2.1.0.5 (Managing Return Content)</remarks>
@@ -157,7 +160,7 @@ public bool PreferCompressedResponses
             set => Settings.PreferCompressedResponses = value;
         }
         /// <summary>
-        /// Compress any Request bodies 
+        /// Compress any Request bodies
         /// (warning, if a server does not handle compressed requests you will get a 415 response)
         /// </summary>
         [Obsolete("Use the FhirClient.Settings property or the settings argument in the constructor instead")]
@@ -201,6 +204,6 @@ protected override void Dispose(bool disposing)
         }
     }
 
-   
+
 }