From f2a57136078de8b1fc59ec2c4a9e98c062d9d19b Mon Sep 17 00:00:00 2001 From: Mateusz Woda Date: Wed, 20 Jul 2022 14:09:38 +0200 Subject: [PATCH] fix: replace infinite retries with exponential backoff strategy in file representations (#835) --- Box.V2.Test/Box.V2.Test.csproj | 6 + Box.V2.Test/BoxFilesManagerTest.cs | 106 +++++++++++++++++- Box.V2.Test/BoxResourceManagerTest.cs | 2 + .../GetRepresentationContentPending200.json | 27 +++++ .../PollRepresentationPending200.json | 17 +++ Box.V2/Config/BoxConfig.cs | 6 + Box.V2/Config/BoxConfigBuilder.cs | 17 +++ Box.V2/Config/IBoxConfig.cs | 5 + Box.V2/Managers/BoxFilesManager.cs | 31 ++++- 9 files changed, 210 insertions(+), 7 deletions(-) create mode 100644 Box.V2.Test/Fixtures/BoxFiles/GetRepresentationContentPending200.json create mode 100644 Box.V2.Test/Fixtures/BoxFiles/PollRepresentationPending200.json diff --git a/Box.V2.Test/Box.V2.Test.csproj b/Box.V2.Test/Box.V2.Test.csproj index 2796b7359..1e1ee6084 100644 --- a/Box.V2.Test/Box.V2.Test.csproj +++ b/Box.V2.Test/Box.V2.Test.csproj @@ -47,6 +47,12 @@ PreserveNewest + + PreserveNewest + + + PreserveNewest + PreserveNewest diff --git a/Box.V2.Test/BoxFilesManagerTest.cs b/Box.V2.Test/BoxFilesManagerTest.cs index d87fca3b2..f941464fc 100644 --- a/Box.V2.Test/BoxFilesManagerTest.cs +++ b/Box.V2.Test/BoxFilesManagerTest.cs @@ -4,7 +4,6 @@ using System.IO; using System.Linq; using System.Net.Http; -using System.Net.Http.Headers; using System.Text; using System.Threading.Tasks; using Box.V2.Exceptions; @@ -1201,5 +1200,110 @@ public async Task DownloadZip_ValidResponse() Assert.AreNotEqual(fs.Length, 0); } } + + + [TestMethod] + [ExpectedException(typeof(BoxCodingException))] + public async Task GetRepresentationContentAsync_ShouldThrowException_IfTooManyRetriesAndHandleRetryTrue() + { + Handler.SetupSequence(h => h.ExecuteAsync(It.IsAny())) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + StatusCode = System.Net.HttpStatusCode.Accepted, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/GetRepresentationContentPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + StatusCode = System.Net.HttpStatusCode.Accepted, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/GetRepresentationContentPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + StatusCode = System.Net.HttpStatusCode.Accepted, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/GetRepresentationContentPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + StatusCode = System.Net.HttpStatusCode.Accepted, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/GetRepresentationContentPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + StatusCode = System.Net.HttpStatusCode.Accepted, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/GetRepresentationContentPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + StatusCode = System.Net.HttpStatusCode.Accepted, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/GetRepresentationContentPending200.json") + })); + + var repRequest = new BoxRepresentationRequest + { + FileId = "11111", + XRepHints = $"[jpg?dimensions=320x320]", + HandleRetry = true + }; + + var result = await _filesManager.GetRepresentationContentAsync(repRequest); + } + + [TestMethod] + [ExpectedException(typeof(BoxCodingException))] + public async Task GetRepresentationContentAsync_ShouldThrowException_IfTooManyRetries() + { + Handler.Setup(h => h.ExecuteAsync(It.IsAny())) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/GetRepresentationContentPending200.json") + })); + + Handler.SetupSequence(h => h.ExecuteAsync(It.IsAny())) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/PollRepresentationPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/PollRepresentationPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/PollRepresentationPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/PollRepresentationPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/PollRepresentationPending200.json") + })) + .Returns(Task.FromResult>(new BoxResponse() + { + Status = ResponseStatus.Success, + ContentString = LoadFixtureFromJson("Fixtures/BoxFiles/PollRepresentationPending200.json") + })); + + var repRequest = new BoxRepresentationRequest + { + FileId = "11111", + XRepHints = $"[jpg?dimensions=320x320]", + }; + + var result = await _filesManager.GetRepresentationContentAsync(repRequest); + } } } diff --git a/Box.V2.Test/BoxResourceManagerTest.cs b/Box.V2.Test/BoxResourceManagerTest.cs index e55ddb1a9..4175edb7d 100644 --- a/Box.V2.Test/BoxResourceManagerTest.cs +++ b/Box.V2.Test/BoxResourceManagerTest.cs @@ -6,6 +6,7 @@ using Box.V2.Converter; using Box.V2.Request; using Box.V2.Services; +using Box.V2.Test.Helpers; using Moq; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -49,6 +50,7 @@ protected BoxResourceManagerTest() Config.SetupGet(x => x.SignRequestsEndpointUri).Returns(SignRequestUri); Config.SetupGet(x => x.SignRequestsEndpointWithPathUri).Returns(SignRequestWithPathUri); Config.SetupGet(x => x.FileRequestsEndpointWithPathUri).Returns(FileRequestsWithPathUri); + Config.SetupGet(x => x.RetryStrategy).Returns(new InstantRetryStrategy()); AuthRepository = new AuthRepository(Config.Object, Service, Converter, new OAuthSession("fakeAccessToken", "fakeRefreshToken", 3600, "bearer")); } diff --git a/Box.V2.Test/Fixtures/BoxFiles/GetRepresentationContentPending200.json b/Box.V2.Test/Fixtures/BoxFiles/GetRepresentationContentPending200.json new file mode 100644 index 000000000..7ed273b21 --- /dev/null +++ b/Box.V2.Test/Fixtures/BoxFiles/GetRepresentationContentPending200.json @@ -0,0 +1,27 @@ +{ + "type": "file", + "id": "11111", + "etag": "0", + "representations": { + "entries": [ + { + "representation": "jpg", + "properties": { + "dimensions": "320x320", + "paged": "false", + "thumb": "false" + }, + "info": { + "url": "https://api.box.com/2.0/internal_files/11111/versions/22222/representations/jpg_320x320" + }, + "status": { + "state": "pending" + }, + "content": { + "url_template": "https://dl.boxcloud.com/api/2.0/internal_files/11111/versions/22222/representations/jpg_320x320/content/{+asset_path}" + } + } + ] + } +} + diff --git a/Box.V2.Test/Fixtures/BoxFiles/PollRepresentationPending200.json b/Box.V2.Test/Fixtures/BoxFiles/PollRepresentationPending200.json new file mode 100644 index 000000000..80b35880a --- /dev/null +++ b/Box.V2.Test/Fixtures/BoxFiles/PollRepresentationPending200.json @@ -0,0 +1,17 @@ +{ + "representation": "jpg", + "properties": { + "dimensions": "320x320", + "paged": "false", + "thumb": "false" + }, + "info": { + "url": "https://api.box.com/2.0/internal_files/11111/versions/22222/representations/jpg_320x320" + }, + "status": { + "state": "pending" + }, + "content": { + "url_template": "https://dl.boxcloud.com/api/2.0/internal_files/11111/versions/22222/representations/jpg_320x320/content/{+asset_path}" + } +} diff --git a/Box.V2/Config/BoxConfig.cs b/Box.V2/Config/BoxConfig.cs index 7ceaf0874..da654bfaf 100644 --- a/Box.V2/Config/BoxConfig.cs +++ b/Box.V2/Config/BoxConfig.cs @@ -66,6 +66,7 @@ public BoxConfig(BoxConfigBuilder builder) AcceptEncoding = builder.AcceptEncoding; WebProxy = builder.WebProxy; Timeout = builder.Timeout; + RetryStrategy = builder.RetryStrategy; } /// @@ -292,6 +293,11 @@ public Uri BoxAuthTokenApiUri /// Timeout for the connection /// public TimeSpan? Timeout { get; private set; } + + /// + /// Retry strategy for failed requests + /// + public IRetryStrategy RetryStrategy { get; private set; } = new ExponentialBackoff(); } public enum CompressionType diff --git a/Box.V2/Config/BoxConfigBuilder.cs b/Box.V2/Config/BoxConfigBuilder.cs index bea252075..e3064ee5c 100644 --- a/Box.V2/Config/BoxConfigBuilder.cs +++ b/Box.V2/Config/BoxConfigBuilder.cs @@ -1,6 +1,7 @@ using System; using System.IO; using System.Net; +using Box.V2.Utility; namespace Box.V2.Config { @@ -251,6 +252,17 @@ public BoxConfigBuilder SetEnterpriseId(string enterpriseId) return this; } + /// + /// Sets retry strategy. + /// + /// Retry strategy. + /// this BoxConfigBuilder object for chaining + public BoxConfigBuilder SetRetryStrategy(IRetryStrategy retryStrategy) + { + RetryStrategy = retryStrategy; + return this; + } + public string ClientId { get; private set; } public string ClientSecret { get; private set; } public string EnterpriseId { get; private set; } @@ -298,6 +310,11 @@ public Uri BoxAuthTokenApiUri /// public TimeSpan? Timeout { get; private set; } + /// + /// Retry strategy for failed requests + /// + public IRetryStrategy RetryStrategy { get; private set; } = new ExponentialBackoff(); + private Uri EnsureEndsWithSlash(Uri uri) { return uri.ToString().EndsWith("/") ? uri : new Uri($"{uri}{"/"}"); diff --git a/Box.V2/Config/IBoxConfig.cs b/Box.V2/Config/IBoxConfig.cs index aee416ad5..30b2b129b 100644 --- a/Box.V2/Config/IBoxConfig.cs +++ b/Box.V2/Config/IBoxConfig.cs @@ -1,5 +1,6 @@ using System; using System.Net; +using Box.V2.Utility; namespace Box.V2.Config { @@ -143,5 +144,9 @@ public interface IBoxConfig /// Timeout for the connection /// TimeSpan? Timeout { get; } + /// + /// Retry strategy for failed requests + /// + IRetryStrategy RetryStrategy { get; } } } diff --git a/Box.V2/Managers/BoxFilesManager.cs b/Box.V2/Managers/BoxFilesManager.cs index 918285543..f999c7b32 100644 --- a/Box.V2/Managers/BoxFilesManager.cs +++ b/Box.V2/Managers/BoxFilesManager.cs @@ -15,6 +15,7 @@ using Box.V2.Extensions; using Box.V2.Models; using Box.V2.Models.Request; +using Box.V2.Request; using Box.V2.Services; using Box.V2.Utility; using Newtonsoft.Json.Linq; @@ -1300,11 +1301,14 @@ public async Task> GetRepresentat IBoxResponse response = await ToResponseAsync(request).ConfigureAwait(false); + var retryCounter = 1; + while (response.StatusCode == HttpStatusCode.Accepted && representationRequest.HandleRetry) { - const int RepresentationRequestRetryTime = 3000; - await Task.Delay(RepresentationRequestRetryTime); - response = await ToResponseAsync(request).ConfigureAwait(false); + response = await CallWithRetryCheck(() => ToResponseAsync(request), + $"Could not get valid File Representation status after {retryCounter} retries.", + retryCounter++) + .ConfigureAwait(false); } return response.ResponseObject.Representations; @@ -1396,7 +1400,7 @@ private async Task CreateZip(BoxZipRequest zipRequest) return response.ResponseObject; } - private async Task PollRepresentationInfo(string infoUrl) + private async Task PollRepresentationInfo(string infoUrl, int retryCounter = 1) { var infoRequest = new BoxRequest(new Uri(infoUrl)); IBoxResponse infoResponse = await ToResponseAsync(infoRequest).ConfigureAwait(false); @@ -1410,12 +1414,27 @@ private async Task PollRepresentationInfo(string infoUrl) throw new BoxCodingException("Representation had error status"); case "none": case "pending": - await Task.Delay(1000); - return await PollRepresentationInfo(infoUrl).ConfigureAwait(false); + return await CallWithRetryCheck(() => PollRepresentationInfo(infoUrl, ++retryCounter), + $"Could not get valid Representation status after {retryCounter} retries.", + retryCounter) + .ConfigureAwait(false); default: throw new BoxCodingException("Representation has unknown status"); } } + + private async Task CallWithRetryCheck(Func> action, string errorMessage, int retryCounter = 1) where T : class + { + if (retryCounter <= HttpRequestHandler.RetryLimit) + { + await Task.Delay(_config.RetryStrategy.GetRetryTimeout(retryCounter)); + return await action().ConfigureAwait(false); + } + else + { + throw new BoxCodingException(errorMessage); + } + } } internal static class UploadUsingSessionInternal