Optimize upload memory usage.

Author: Qiming Yuan

Dropbox.Api.Tests/DropboxApiTests.cs

@@ -24,6 +24,11 @@ namespace Dropbox.Api.Tests
     [TestClass]
     public class DropboxApiTests
     {
+        /// <summary>
+        /// The user access token.
+        /// </summary>
+        public static string UserAccessToken;
+
         /// <summary>
         /// The Dropbox client.
         /// </summary>
@@ -39,11 +44,12 @@ public class DropboxApiTests
         /// </summary>
         public static DropboxAppClient AppClient;
 
+
         [ClassInitialize]
         public static void Initialize(TestContext context)
         {
-            var userToken = context.Properties["userAccessToken"].ToString();
-            Client = new DropboxClient(userToken);
+            UserAccessToken = context.Properties["userAccessToken"].ToString();
+            Client = new DropboxClient(UserAccessToken);
 
             var teamToken = context.Properties["teamAccessToken"].ToString();
             TeamClient = new DropboxTeamClient(teamToken);
@@ -123,6 +129,42 @@ public async Task TestUpload()
             Assert.AreEqual("abc", content);
         }
 
+        /// <summary>
+        /// Test upload with retry.
+        /// </summary>
+        /// <returns>The <see cref="Task"/></returns>
+        [TestMethod]
+        public async Task TestUploadRetry()
+        {
+            var count = 0;
+
+            var mockHandler = new MockHttpMessageHandler((r, s) =>
+            {
+                if (count++ < 2)
+                {
+                    var error = new HttpResponseMessage(HttpStatusCode.InternalServerError)
+                    {
+                        Content = new StringContent("Error")
+                    };
+
+                    return Task.FromResult(error);
+                }
+
+                return s(r);
+            });
+
+            var mockClient = new HttpClient(mockHandler);
+            var client = new DropboxClient(
+                UserAccessToken,
+                new DropboxClientConfig { HttpClient = mockClient, MaxRetriesOnError = 10 });
+
+            var response = await client.Files.UploadAsync("/Foo.txt", body: GetStream("abc"));
+            var downloadResponse = await Client.Files.DownloadAsync("/Foo.txt");
+            var content = await downloadResponse.GetContentAsStringAsync();
+            Assert.AreEqual("abc", content);
+        }
+
+
         /// <summary>
         /// Test upload.
         /// </summary>
@@ -154,7 +196,7 @@ public async Task TestRateLimit()
 
             mockResponse.Headers.Add("X-Dropbox-Request-Id", "123");
 
-            var mockHandler = new MockHttpMessageHandler(mockResponse);
+            var mockHandler = new MockHttpMessageHandler((r, s) => Task.FromResult(mockResponse));
             var mockClient = new HttpClient(mockHandler);
             var client = new DropboxClient("dummy", new DropboxClientConfig { HttpClient = mockClient });
             try
@@ -275,5 +317,44 @@ private static MemoryStream GetStream(string content)
             var buffer = Encoding.UTF8.GetBytes(content);
             return new MemoryStream(buffer);
         }
+        
+        /// Test User-Agent header is set with default values.
+        /// </summary>
+        /// <returns>The <see cref="Task"/></returns>
+        [TestMethod]
+        public async Task TestUserAgentDefault()
+        {
+            HttpRequestMessage lastRequest = null;
+            var mockHandler = new MockHttpMessageHandler((r, s) =>
+            {
+                lastRequest = r;
+                return s(r);
+            });
+
+            var mockClient = new HttpClient(mockHandler);
+            var client = new DropboxClient(UserAccessToken, new DropboxClientConfig { HttpClient = mockClient });
+            await client.Users.GetCurrentAccountAsync();
+            Assert.IsTrue(lastRequest.Headers.UserAgent.ToString().Contains("OfficialDropboxDotNetSDKv2"));
+        }
+
+        /// Test User-Agent header is populated with user supplied value in DropboxClientConfig.
+        /// </summary>
+        /// <returns>The <see cref="Task"/></returns>
+        [TestMethod]
+        public async Task TestUserAgentUserSupplied()
+        {
+            HttpRequestMessage lastRequest = null;
+            var mockHandler = new MockHttpMessageHandler((r, s) =>
+            {
+                lastRequest = r;
+                return s(r);
+            });
+
+            var mockClient = new HttpClient(mockHandler);
+            var userAgent = "UserAgentTest";
+            var client = new DropboxClient(UserAccessToken, new DropboxClientConfig { HttpClient = mockClient, UserAgent = userAgent });
+            await client.Users.GetCurrentAccountAsync();
+            Assert.IsTrue(lastRequest.Headers.UserAgent.ToString().Contains(userAgent));
+        }
     }
 }

Dropbox.Api.Tests/MockHttpMessageHandler.cs

@@ -6,24 +6,27 @@
 
 namespace Dropbox.Api.Tests
 {
+    using System;
     using System.Net.Http;
     using System.Threading;
     using System.Threading.Tasks;
 
-    public class MockHttpMessageHandler : HttpMessageHandler
+    public class MockHttpMessageHandler : HttpClientHandler
     {
+        public delegate Task<HttpResponseMessage> Sender(HttpRequestMessage message);
+
         /// <summary>
-        /// The fake response.
+        /// The mock handler.
         /// </summary>
-        private readonly HttpResponseMessage response;
-        
+        Func<HttpRequestMessage, Sender, Task<HttpResponseMessage>> handler;
+
         /// <summary>
         /// Initializes a new instance of the <see cref="MockHttpMessageHandler"/> class.
         /// </summary>
         /// <param name="response">The mock response.</param>
-        public MockHttpMessageHandler(HttpResponseMessage response)
+        public MockHttpMessageHandler(Func<HttpRequestMessage, Sender, Task<HttpResponseMessage>> handler)
         {
-            this.response = response;
+            this.handler = handler;
         }
 
         /// <summary>
@@ -34,7 +37,7 @@ public MockHttpMessageHandler(HttpResponseMessage response)
         /// <returns>The response.</returns>
         protected override Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
         {
-            return Task.FromResult(this.response);
+            return this.handler(request, r => base.SendAsync(r, cancellationToken));
         }
     }
 }

Dropbox.Api/DropboxRequestHandler.cs

@@ -238,9 +238,6 @@ private async Task<Result> RequestJsonStringWithRetry(
             var maxRetries = this.options.MaxClientRetries;
             var r = new Random();
 
-            byte[] cachedBody = null;
-            long cachedStreamStart = 0;
-
             if (routeStyle == RouteStyle.Upload)
             {
                 if (body == null)
@@ -254,66 +251,50 @@ private async Task<Result> RequestJsonStringWithRetry(
                 {
                     maxRetries = 0;
                 }
-                else if (maxRetries == 0)
-                {
-                    // Do not copy the stream
-                }
-                else if (body is MemoryStream)
-                {
-                    cachedStreamStart = body.Position;
-                    cachedBody = ((MemoryStream)body).ToArray();
-                }
-                else
-                {
-                    cachedStreamStart = body.Position;
-                    using (var mem = new MemoryStream())
-                    {
-                        await body.CopyToAsync(mem).ConfigureAwait(false);
-                        cachedBody = mem.ToArray();
-                    }
-                }
             }
 
-            while (true)
+            try
             {
-                try
+                while (true)
                 {
-                    if (cachedBody == null)
+                    try
                     {
                         return await this.RequestJsonString(host, routeName, auth, routeStyle, requestArg, body)
                             .ConfigureAwait(false);
                     }
-                    else
+                    catch (RateLimitException)
                     {
-                        using (var mem = new MemoryStream(cachedBody, writable: false))
-                        {
-                            mem.Position = cachedStreamStart;
-                            return await this.RequestJsonString(host, routeName, auth, routeStyle, requestArg, mem)
-                                .ConfigureAwait(false);
-                        }
+                        throw;
                     }
-                }
-                catch (RateLimitException)
-                {
-                    throw;
-                }
-                catch (RetryException)
-                {
-                    // dropbox maps 503 - ServiceUnavailable to be a rate limiting error.
-                    // do not count a rate limiting error as an attempt
-                    if (++attempt > maxRetries)
+                    catch (RetryException)
                     {
-                        throw;
+                        // dropbox maps 503 - ServiceUnavailable to be a rate limiting error.
+                        // do not count a rate limiting error as an attempt
+                        if (++attempt > maxRetries)
+                        {
+                            throw;
+                        }
                     }
-                }
 
-                // use exponential backoff
-                var backoff = TimeSpan.FromSeconds(Math.Pow(2, attempt) * r.NextDouble());
+                    // use exponential backoff
+                    var backoff = TimeSpan.FromSeconds(Math.Pow(2, attempt) * r.NextDouble());
 #if PORTABLE40
-                await TaskEx.Delay(backoff);
+                    await TaskEx.Delay(backoff);
 #else
-                await Task.Delay(backoff);
+                    await Task.Delay(backoff);
 #endif
+                    if (body != null)
+                    {
+                        body.Position = 0;
+                    }
+                }
+            }
+            finally
+            {
+                if (body != null)
+                {
+                    body.Dispose();
+                }
             }
         }
 
@@ -412,7 +393,7 @@ private async Task<Result> RequestJsonString(
                         throw new ArgumentNullException("body");
                     }
 
-                    request.Content = new StreamContent(body);
+                    request.Content = new CustomStreamContent(body);
                     request.Content.Headers.ContentType = new MediaTypeHeaderValue("application/octet-stream");
                     break;
                 default:
@@ -641,6 +622,26 @@ public void Dispose()
         }
     }
 
+    /// <summary>
+    /// The stream content which doesn't dispose the underlying stream. This
+    /// is useful for retry.
+    /// </summary>
+    internal class CustomStreamContent : StreamContent
+    {
+        /// <summary>
+        /// Initializes a new instance of the <see cref="CustomStreamContent"/> class.
+        /// </summary>
+        /// <param name="content">The stream content.</param>
+        public CustomStreamContent(Stream content) : base(content)
+        {
+        }
+
+        protected override void Dispose(bool disposing)
+        {
+            // Do not dispose the stream.
+        }
+    }
+
     /// <summary>
     /// The type of api hosts.
     /// </summary>
@@ -752,7 +753,7 @@ public DropboxRequestHandlerOptions(
 
             this.UserAgent = userAgent == null
                 ? string.Join("/", BaseUserAgent, sdkVersion)
-                : string.Join("/", this.UserAgent, BaseUserAgent, sdkVersion);
+                : string.Join("/", userAgent, BaseUserAgent, sdkVersion);
 
             this.HttpClient = httpClient ?? DefaultHttpClient;
             this.OAuth2AccessToken = oauth2AccessToken;