Merge pull request #810 from octokit/fix-redirect-with-archive-link

finish support for downloading the code archive

Octokit.Reactive/Clients/IObservableRepositoryContentsClient.cs

@@ -34,8 +34,18 @@ namespace Octokit.Reactive
         /// <param name="owner">The owner of the repository</param>
         /// <param name="name">The name of the repository</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         IObservable<string> GetArchiveLink(string owner, string name);
 
+        /// <summary>
+        /// Get an archive of a given repository's contents
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <returns>A promise, containing the binary contents of the archive</returns>
+        IObservable<byte[]> GetArchive(string owner, string name);
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -47,8 +57,19 @@ namespace Octokit.Reactive
         /// <param name="name">The name of the repository</param>
         /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat);
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, in a specific format
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <returns>A promise, containing the binary contents of the archive</returns>
+        IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat);
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -61,8 +82,20 @@ namespace Octokit.Reactive
         /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
         /// <param name="reference">A valid Git reference.</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference);
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, using a specific format and reference
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <param name="reference">A valid Git reference.</param>
+        /// <returns>A promise, containing the binary contents of the archive</returns>
+        IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference);
+
         /// <summary>
         /// Returns the contents of a file or directory in a repository.
         /// </summary>

Octokit.Reactive/Clients/ObservableRepositoryContentsClient.cs

@@ -59,11 +59,24 @@ namespace Octokit.Reactive
         /// <param name="owner">The owner of the repository</param>
         /// <param name="name">The name of the repository</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         public IObservable<string> GetArchiveLink(string owner, string name)
         {
             return GetArchiveLink(owner, name, ArchiveFormat.Tarball, string.Empty);
         }
 
+        /// <summary>
+        /// Get an archive of a given repository's contents
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <returns>A promise, containing the binary contents of the archive</returns>
+        public IObservable<byte[]> GetArchive(string owner, string name)
+        {
+            return _client.Repository.Content.GetArchive(owner, name).ToObservable();
+        }
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -75,11 +88,25 @@ namespace Octokit.Reactive
         /// <param name="name">The name of the repository</param>
         /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         public IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat)
         {
             return GetArchiveLink(owner, name, archiveFormat, String.Empty);
         }
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, in a specific format
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <returns>A promise, containing the binary contents of the archive</returns>
+        public IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat)
+        {
+            return _client.Repository.Content.GetArchive(owner, name, archiveFormat).ToObservable();
+        }
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -92,6 +119,7 @@ namespace Octokit.Reactive
         /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
         /// <param name="reference">A valid Git reference.</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         public IObservable<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference)
         {
             Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
@@ -100,6 +128,20 @@ namespace Octokit.Reactive
             return _client.Repository.Content.GetArchiveLink(owner, name, archiveFormat, reference).ToObservable();
         }
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, using a specific format and reference
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <param name="reference">A valid Git reference.</param>
+        /// <returns>A promise, containing the binary contents of the archive</returns>
+        public IObservable<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference)
+        {
+            return _client.Repository.Content.GetArchive(owner, name, archiveFormat, reference).ToObservable();
+        }
+
         /// <summary>
         /// Returns the contents of a file or directory in a repository.
         /// </summary>

Octokit.Tests.Integration/Clients/RepositoryContentsClientTests.cs

@@ -115,43 +115,43 @@ namespace Octokit.Tests.Integration.Clients
             }
         }
 
-        [IntegrationTest]
-        public async Task GetsArchiveLinkAsTarball()
+        [IntegrationTest(Skip = "this will probably take too long")]
+        public async Task GetsArchiveAsTarball()
         {
             var github = Helper.GetAuthenticatedClient();
 
-            var archiveLink = await github
+            var archive = await github
                 .Repository
                 .Content
-                .GetArchiveLink("octokit", "octokit.net");
+                .GetArchive("octokit", "octokit.net");
 
-            Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.tar.gz/master", archiveLink);
+            Assert.NotEmpty(archive);
         }
 
         [IntegrationTest]
-        public async Task GetsArchiveLinkAsZipball()
+        public async Task GetsArchiveAsZipball()
         {
             var github = Helper.GetAuthenticatedClient();
 
-            var archiveLink = await github
+            var archive = await github
                 .Repository
                 .Content
-                .GetArchiveLink("octokit", "octokit.net", ArchiveFormat.Zipball, "");
+                .GetArchive("octokit", "octokit.net", ArchiveFormat.Zipball, "");
 
-            Assert.Equal("https://codeload.github.com/octokit/octokit.net/legacy.zip/master", archiveLink);
+            Assert.NotEmpty(archive);
         }
 
         [IntegrationTest]
-        public async Task GetsArchiveLinkForReleaseBranchAsTarball()
+        public async Task GetsArchiveForReleaseBranchAsTarball()
         {
             var github = Helper.GetAuthenticatedClient();
 
-            var archiveLink = await github
+            var archive = await github
                 .Repository
                 .Content
-                .GetArchiveLink("alfhenrik", "ScriptCs.OctoKit", ArchiveFormat.Tarball, "dev");
+                .GetArchive("alfhenrik", "ScriptCs.OctoKit", ArchiveFormat.Tarball, "dev");
 
-            Assert.Equal("https://codeload.github.com/alfhenrik/ScriptCs.OctoKit/legacy.tar.gz/dev", archiveLink);
+            Assert.NotEmpty(archive);
         }
     }
 }

Octokit.Tests/Http/RedirectHandlerTests.cs

@@ -102,18 +102,6 @@ namespace Octokit.Tests.Http
             Assert.Null(response.RequestMessage.Headers.Authorization);
 
         }
-        
-        [Fact]
-        public async Task DisabledRedirectShouldPassThrough()
-        {
-            var invoker = CreateInvoker(new HttpResponseMessage(HttpStatusCode.Found));
-            var httpRequestMessage = CreateRequest(HttpMethod.Get);
-            httpRequestMessage.Properties[RedirectHandler.AllowAutoRedirectKey] = false;
-            var response = await invoker.SendAsync(httpRequestMessage, new CancellationToken());
-
-            Assert.Equal(response.StatusCode, HttpStatusCode.Redirect);
-            Assert.Same(response.RequestMessage, httpRequestMessage);
-        }
 
         [Theory]
         [InlineData(HttpStatusCode.MovedPermanently)]  // 301
@@ -179,7 +167,6 @@ namespace Octokit.Tests.Http
         {
             var httpRequestMessage = new HttpRequestMessage();
             httpRequestMessage.RequestUri = new Uri("http://example.org/foo");
-            httpRequestMessage.Properties[RedirectHandler.AllowAutoRedirectKey] = true;
             httpRequestMessage.Method = method;
             return httpRequestMessage;
         }

Octokit/Clients/IRepositoryContentsClient.cs

@@ -1,3 +1,4 @@
+using System;
 using System.Collections.Generic;
 using System.Threading.Tasks;
 using Octokit.Internal;
@@ -57,8 +58,18 @@ namespace Octokit
         /// <param name="owner">The owner of the repository</param>
         /// <param name="name">The name of the repository</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         Task<string> GetArchiveLink(string owner, string name);
 
+        /// <summary>
+        /// Get an archive of a given repository's contents
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <returns>The binary contents of the archive</returns>
+        Task<byte[]> GetArchive(string owner, string name);
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -70,8 +81,19 @@ namespace Octokit
         /// <param name="name">The name of the repository</param>
         /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         Task<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat);
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, in a specific format
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <returns>The binary contents of the archive</returns>
+        Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat);
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -84,8 +106,20 @@ namespace Octokit
         /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
         /// <param name="reference">A valid Git reference.</param>
         /// <returns></returns>
+        [Obsolete("Use GetArchive to download the archive instead")]
         Task<string> GetArchiveLink(string owner, string name, ArchiveFormat archiveFormat, string reference);
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, using a specific format and reference
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <param name="reference">A valid Git reference.</param>
+        /// <returns>The binary contents of the archive</returns>
+        Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference);
+
         /// <summary>
         /// Creates a commit that creates a new file in a repository.
         /// </summary>

Octokit/Clients/RepositoryContentsClient.cs

@@ -89,6 +89,18 @@ namespace Octokit
             return GetArchiveLink(owner, name, ArchiveFormat.Tarball, string.Empty);
         }
 
+        /// <summary>
+        /// Get an archive of a given repository's contents
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <returns>The binary contents of the archive</returns>
+        public Task<byte[]> GetArchive(string owner, string name)
+        {
+            return GetArchive(owner, name, ArchiveFormat.Tarball, string.Empty);
+        }
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -105,6 +117,19 @@ namespace Octokit
             return GetArchiveLink(owner, name, archiveFormat, string.Empty);
         }
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, in a specific format
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <returns>The binary contents of the archive</returns>
+        public Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat)
+        {
+            return GetArchive(owner, name, archiveFormat, string.Empty);
+        }
+
         /// <summary>
         /// This method will return a 302 to a URL to download a tarball or zipball archive for a repository.
         /// Please make sure your HTTP framework is configured to follow redirects or you will need to use the 
@@ -125,6 +150,27 @@ namespace Octokit
             return ApiConnection.GetRedirect(ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference));
         }
 
+        /// <summary>
+        /// Get an archive of a given repository's contents, using a specific format and reference
+        /// </summary>
+        /// <remarks>https://developer.github.com/v3/repos/contents/#get-archive-link</remarks>
+        /// <param name="owner">The owner of the repository</param>
+        /// <param name="name">The name of the repository</param>
+        /// <param name="archiveFormat">The format of the archive. Can be either tarball or zipball</param>
+        /// <param name="reference">A valid Git reference.</param>
+        /// <returns>The binary contents of the archive</returns>
+        public async Task<byte[]> GetArchive(string owner, string name, ArchiveFormat archiveFormat, string reference)
+        {
+            Ensure.ArgumentNotNullOrEmptyString(owner, "owner");
+            Ensure.ArgumentNotNullOrEmptyString(name, "name");
+
+            var endpoint = ApiUrls.RepositoryArchiveLink(owner, name, archiveFormat, reference);
+
+            var response = await Connection.Get<byte[]>(endpoint, new Dictionary<string, string>(), null);
+
+            return response.Body;
+        }
+
         /// <summary>
         /// Creates a commit that creates a new file in a repository.
         /// </summary>

Octokit/Http/Connection.cs

@@ -153,12 +153,12 @@ namespace Octokit
         /// <param name="accepts">Specifies accepted response media types.</param>
         /// <param name="allowAutoRedirect">To follow redirect links automatically or not</param>
         /// <returns><seealso cref="IResponse"/> representing the received HTTP response</returns>
-
+        [Obsolete("allowAutoRedirect is no longer respected and will be deprecated in a future release")]
         public Task<IApiResponse<T>> Get<T>(Uri uri, IDictionary<string, string> parameters, string accepts, bool allowAutoRedirect)
         {
             Ensure.ArgumentNotNull(uri, "uri");
 
-            return SendData<T>(uri.ApplyParameters(parameters), HttpMethod.Get, null, accepts, null, CancellationToken.None, allowAutoRedirect: allowAutoRedirect);
+            return SendData<T>(uri.ApplyParameters(parameters), HttpMethod.Get, null, accepts, null, CancellationToken.None);
         }
 
         public Task<IApiResponse<T>> Get<T>(Uri uri, IDictionary<string, string> parameters, string accepts, CancellationToken cancellationToken)
@@ -321,8 +321,7 @@ namespace Octokit
             string contentType,
             CancellationToken cancellationToken,
             string twoFactorAuthenticationCode = null,
-            Uri baseAddress = null,
-            bool allowAutoRedirect = true)
+            Uri baseAddress = null)
         {
             Ensure.ArgumentNotNull(uri, "uri");
 
@@ -331,7 +330,6 @@ namespace Octokit
                 Method = method,
                 BaseAddress = baseAddress ?? BaseAddress,
                 Endpoint = uri,
-                AllowAutoRedirect = allowAutoRedirect,
             };
 
             return SendDataInternal<T>(body, accepts, contentType, cancellationToken, twoFactorAuthenticationCode, request);

Octokit/Http/HttpClientAdapter.cs

@@ -77,7 +77,9 @@ namespace Octokit.Internal
                     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.Equals("application/zip", StringComparison.OrdinalIgnoreCase)))
+                    if (contentType != null && (contentType.StartsWith("image/")
+                        || contentType.Equals("application/zip", StringComparison.OrdinalIgnoreCase)
+                        || contentType.Equals("application/x-gzip", StringComparison.OrdinalIgnoreCase)))
                     {
                         responseBody = await responseMessage.Content.ReadAsByteArrayAsync().ConfigureAwait(false);
                     }
@@ -104,7 +106,6 @@ namespace Octokit.Internal
                 var fullUri = new Uri(request.BaseAddress, request.Endpoint);
                 requestMessage = new HttpRequestMessage(request.Method, fullUri);
 
-                requestMessage.Properties["AllowAutoRedirect"] = request.AllowAutoRedirect;
                 foreach (var header in request.Headers)
                 {
                     requestMessage.Headers.Add(header.Key, header.Value);
@@ -166,7 +167,6 @@ namespace Octokit.Internal
 
     internal class RedirectHandler : DelegatingHandler
     {
-        public const string AllowAutoRedirectKey = "AllowAutoRedirect";
         public const string RedirectCountKey = "RedirectCount";
         public bool Enabled { get; set; }
 
@@ -177,10 +177,6 @@ namespace Octokit.Internal
             // Can't redirect without somewhere to redirect too.  Throw?
             if (response.Headers.Location == null) return response;
 
-            // Don't redirect if redirection has been disabled for this request
-            var allowAutoRedirect = (bool)request.Properties[AllowAutoRedirectKey];
-            if (!allowAutoRedirect) return response; // Throw?
-
             // Don't redirect if we exceed max number of redirects
             var redirectCount = 0;
             if (request.Properties.Keys.Contains(RedirectCountKey))

Octokit/Http/IRequest.cs

@@ -14,6 +14,7 @@ namespace Octokit.Internal
         Uri Endpoint { get; }
         TimeSpan Timeout { get; }
         string ContentType { get; }
+        [Obsolete("This value is no longer respected due to the necessary redirect work")]
         bool AllowAutoRedirect { get; }
     }
 }

Octokit/Http/Request.cs

@@ -11,7 +11,6 @@ namespace Octokit.Internal
         {
             Headers = new Dictionary<string, string>();
             Parameters = new Dictionary<string, string>();
-            AllowAutoRedirect = true;
             Timeout = TimeSpan.FromSeconds(100);
         }
 
@@ -23,6 +22,8 @@ namespace Octokit.Internal
         public Uri Endpoint { get; set; }
         public TimeSpan Timeout { get; set; }
         public string ContentType { get; set; }
+
+        [Obsolete("This value is no longer respected due to the necessary redirect work")]
         public bool AllowAutoRedirect { get; set; }
     }
 }