Merge pull request #796 from octokit/deprecate-httpclient-timeout

deprecate httpclient timeout in favour of cancellation token
octokit · May 8, 2015 · e6ce664 · e6ce664
2 parents 54c5762 + 1772904
commit e6ce664
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 10 deletions.
diff --git a/Octokit.Tests.Integration/HttpClientAdapterTests.cs b/Octokit.Tests.Integration/HttpClientAdapterTests.cs
@@ -32,5 +32,25 @@ public async Task CanDownloadImage()
             Assert.Equal(78, imageBytes[2]);
             Assert.Equal(130, imageBytes.Last());
         }
+
+        [IntegrationTest]
+        public async Task CanCancelARequest()
+        {
+            var httpClient = new HttpClientAdapter();
+            var request = new Request
+            {
+                BaseAddress = new Uri("https://github.global.ssl.fastly.net/", UriKind.Absolute),
+                Endpoint = new Uri("/images/icons/emoji/poop.png?v=5", UriKind.RelativeOrAbsolute),
+                AllowAutoRedirect = true,
+                Method = HttpMethod.Get,
+                Timeout = TimeSpan.FromMilliseconds(10)
+            };
+
+            var response = httpClient.Send(request, CancellationToken.None);
+
+            await Task.Delay(TimeSpan.FromSeconds(2));
+
+            Assert.True(response.IsCanceled);
+        }
     }
 }
diff --git a/Octokit.Tests/Http/HttpClientAdapterTests.cs b/Octokit.Tests/Http/HttpClientAdapterTests.cs
@@ -16,11 +16,15 @@ public class HttpClientAdapterTests
     {
         public class TheBuildRequestMessageMethod
         {
+            readonly Uri _endpoint = new Uri("/ha-ha-business", UriKind.Relative);
+
             [Fact]
             public void AddsHeadersToRequestMessage()
             {
                 var request = new Request
                 {
+                    BaseAddress = GitHubClient.GitHubApiUrl,
+                    Endpoint = _endpoint,
                     Method = HttpMethod.Post,
                     Headers =
                     {
@@ -47,6 +51,8 @@ public void SetsBodyAndContentType()
             {
                 var request = new Request
                 {
+                    BaseAddress = GitHubClient.GitHubApiUrl,
+                    Endpoint = _endpoint,
                     Method = HttpMethod.Post,
                     Body = "{}",
                     ContentType = "text/plain"
@@ -64,6 +70,8 @@ public void SetsStreamBodyAndContentType()
             {
                 var request = new Request
                 {
+                    BaseAddress = GitHubClient.GitHubApiUrl,
+                    Endpoint = _endpoint,
                     Method = HttpMethod.Post,
                     Body = new MemoryStream(),
                     ContentType = "text/plain"
@@ -82,6 +90,8 @@ public void SetsHttpContentBody()
             {
                 var request = new Request
                 {
+                    BaseAddress = GitHubClient.GitHubApiUrl,
+                    Endpoint = _endpoint,
                     Method = HttpMethod.Post,
                     Body = new FormUrlEncodedContent(new Dictionary<string, string> {{"foo", "bar"}})
                 };

diff --git a/Octokit/GitHubClient.cs b/Octokit/GitHubClient.cs
@@ -23,7 +23,7 @@ public class GitHubClient : IGitHubClient
         /// the user agent for analytics purposes.
         /// </param>
         public GitHubClient(ProductHeaderValue productInformation)
-            : this(new Connection(productInformation))
+            : this(new Connection(productInformation, GitHubApiUrl))
         {
         }
 

diff --git a/Octokit/Http/HttpClientAdapter.cs b/Octokit/Http/HttpClientAdapter.cs
@@ -50,15 +50,21 @@ public async Task<IResponse> Send(IRequest request, CancellationToken cancellati
                 httpOptions.Proxy = _webProxy;
             }
 
-            var http = new HttpClient(httpOptions)
+            var http = new HttpClient(httpOptions);
+            var cancellationTokenForRequest = cancellationToken;
+
+            if (request.Timeout != TimeSpan.Zero)
             {
-                BaseAddress = request.BaseAddress,
-                Timeout = request.Timeout
-            };
+                var timeoutCancellation = new CancellationTokenSource(request.Timeout);
+                var unifiedCancellationToken = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCancellation.Token);
+
+                cancellationTokenForRequest = unifiedCancellationToken.Token;
+            }
+
             using (var requestMessage = BuildRequestMessage(request))
             {
                 // Make the request
-                var responseMessage = await http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationToken)
+                var responseMessage = await http.SendAsync(requestMessage, HttpCompletionOption.ResponseContentRead, cancellationTokenForRequest)
                                                 .ConfigureAwait(false);
                 return await BuildResponse(responseMessage).ConfigureAwait(false);
             }
@@ -77,13 +83,13 @@ protected async virtual Task<IResponse> BuildResponse(HttpResponseMessage respon
                     contentType = GetContentMediaType(responseMessage.Content);
 
                     // We added support for downloading images and zip-files. Let's constrain this appropriately.
-                    if (contentType == null || (!contentType.StartsWith("image/") && !contentType.StartsWith("application/")))
+                    if (contentType != null && (contentType.StartsWith("image/") || contentType.Equals("application/zip", StringComparison.OrdinalIgnoreCase)))
                     {
-                        responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false);
+                        responseBody = await responseMessage.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
                     }
                     else
                     {
-                        responseBody = await responseMessage.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
+                        responseBody = await responseMessage.Content.ReadAsStringAsync().ConfigureAwait(false);
                     }
                 }
             }
@@ -101,7 +107,8 @@ protected virtual HttpRequestMessage BuildRequestMessage(IRequest request)
             HttpRequestMessage requestMessage = null;
             try
             {
-                requestMessage = new HttpRequestMessage(request.Method, request.Endpoint);
+                var fullUri = new Uri(request.BaseAddress, request.Endpoint);
+                requestMessage = new HttpRequestMessage(request.Method, fullUri);
                 foreach (var header in request.Headers)
                 {
                     requestMessage.Headers.Add(header.Key, header.Value);