From 1f00e51ab0dc6b6b303b612f83510e44cf55a461 Mon Sep 17 00:00:00 2001 From: Finn Reilly Date: Thu, 9 Jan 2025 10:41:30 +0000 Subject: [PATCH 1/3] fix issue with RateLimits where burst was reached * time comparison was incorrect * makes private method names less confusing --- Amazon-SP-API-CSharp.sln | 7 ++++ Source/FikaAmazonAPI.SampleCode/Program.cs | 2 +- Source/FikaAmazonAPI/Utils/RateLimits.cs | 16 ++++----- Source/Tests/GlobalUsings.cs | 1 + Source/Tests/RateLimitsTests.cs | 40 ++++++++++++++++++++++ Source/Tests/Tests.csproj | 24 +++++++++++++ 6 files changed, 81 insertions(+), 9 deletions(-) create mode 100644 Source/Tests/GlobalUsings.cs create mode 100644 Source/Tests/RateLimitsTests.cs create mode 100644 Source/Tests/Tests.csproj diff --git a/Amazon-SP-API-CSharp.sln b/Amazon-SP-API-CSharp.sln index 5341cd64..cf60c551 100644 --- a/Amazon-SP-API-CSharp.sln +++ b/Amazon-SP-API-CSharp.sln @@ -18,6 +18,8 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "FikaAmazonAPI.SampleCode", EndProject Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "FikaAmazonAPI", "Source\FikaAmazonAPI\FikaAmazonAPI.csproj", "{D6BE954D-174D-4C19-A0B6-46F020878E72}" EndProject +Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Tests", "Source\Tests\Tests.csproj", "{4CB44101-8A9E-454A-B272-038C5FAF9F23}" +EndProject Global GlobalSection(SolutionConfigurationPlatforms) = preSolution Debug|Any CPU = Debug|Any CPU @@ -32,6 +34,10 @@ Global {D6BE954D-174D-4C19-A0B6-46F020878E72}.Debug|Any CPU.Build.0 = Debug|Any CPU {D6BE954D-174D-4C19-A0B6-46F020878E72}.Release|Any CPU.ActiveCfg = Release|Any CPU {D6BE954D-174D-4C19-A0B6-46F020878E72}.Release|Any CPU.Build.0 = Release|Any CPU + {4CB44101-8A9E-454A-B272-038C5FAF9F23}.Debug|Any CPU.ActiveCfg = Debug|Any CPU + {4CB44101-8A9E-454A-B272-038C5FAF9F23}.Debug|Any CPU.Build.0 = Debug|Any CPU + {4CB44101-8A9E-454A-B272-038C5FAF9F23}.Release|Any CPU.ActiveCfg = Release|Any CPU + {4CB44101-8A9E-454A-B272-038C5FAF9F23}.Release|Any CPU.Build.0 = Release|Any CPU EndGlobalSection GlobalSection(SolutionProperties) = preSolution HideSolutionNode = FALSE @@ -39,6 +45,7 @@ Global GlobalSection(NestedProjects) = preSolution {FC494085-19C4-4835-B053-F9B74FFB978A} = {3472E85C-6C29-4196-AA16-B95898241C04} {D6BE954D-174D-4C19-A0B6-46F020878E72} = {3472E85C-6C29-4196-AA16-B95898241C04} + {4CB44101-8A9E-454A-B272-038C5FAF9F23} = {3472E85C-6C29-4196-AA16-B95898241C04} EndGlobalSection GlobalSection(ExtensibilityGlobals) = postSolution SolutionGuid = {F072E7DD-BF35-43CC-BF83-E5947CA2D772} diff --git a/Source/FikaAmazonAPI.SampleCode/Program.cs b/Source/FikaAmazonAPI.SampleCode/Program.cs index fad28a28..647ed438 100644 --- a/Source/FikaAmazonAPI.SampleCode/Program.cs +++ b/Source/FikaAmazonAPI.SampleCode/Program.cs @@ -20,7 +20,7 @@ static async Task Main(string[] args) RefreshToken: config.GetSection("FikaAmazonAPI:RefreshToken").Value, rateLimitingHandler: new RateLimitingHandler()); - var tasks = new[] { 1..10 }.Select(x => + var tasks = new[] { 1..80 }.Select(x => Task.Run(() => { var amazonConnection = connectionFactory.RequestScopedConnection( diff --git a/Source/FikaAmazonAPI/Utils/RateLimits.cs b/Source/FikaAmazonAPI/Utils/RateLimits.cs index 5237b4d3..65f55c72 100644 --- a/Source/FikaAmazonAPI/Utils/RateLimits.cs +++ b/Source/FikaAmazonAPI/Utils/RateLimits.cs @@ -1,8 +1,9 @@ using System; -using System.Runtime; +using System.Runtime.CompilerServices; using System.Threading; using System.Threading.Tasks; +[assembly: InternalsVisibleTo("Tests")] namespace FikaAmazonAPI.Utils { internal class RateLimits @@ -47,10 +48,9 @@ public async Task WaitForPermittedRequest(CancellationToken cancellationToken = } int ratePeriodMs = GetRatePeriodMs(); - var requestsUsedAtOnset = RequestsUsed; // when requests used more than zero, replenish according to time elapsed - IncrementAvailableTokens(debugMode); + DecrementRequestsUsed(debugMode); var nextRequestsSent = RequestsUsed + 1; var nextRequestsSentTxt = (nextRequestsSent > Burst) ? "FULL" : nextRequestsSent.ToString(); @@ -76,7 +76,7 @@ public async Task WaitForPermittedRequest(CancellationToken cancellationToken = else { // replenish token - IncrementAvailableTokens(debugMode); + DecrementRequestsUsed(debugMode); } if (RequestsUsed <= 0) @@ -87,7 +87,7 @@ public async Task WaitForPermittedRequest(CancellationToken cancellationToken = } // now remove token from bucket for pending request - requestIsPermitted = TryDecrementAvailableTokens(debugMode); + requestIsPermitted = TryIncrementRequestsUsed(debugMode); } } @@ -97,13 +97,13 @@ internal void SetRateLimit(decimal rate) } // increments available tokens, unless another thread has already incremented them. - private void IncrementAvailableTokens(bool isDebug) + private void DecrementRequestsUsed(bool isDebug) { WriteDebug($"Attempting to increment tokens", isDebug); lock (_locker) { var ratePeriodMilliseconds = GetRatePeriodMs(); - var requestsToReplenish = ratePeriodMilliseconds == 0 ? 0 : (DateTime.UtcNow - LastRequestReplenished).Milliseconds / ratePeriodMilliseconds; + var requestsToReplenish = ratePeriodMilliseconds == 0 ? 0 : (int)((DateTime.UtcNow - LastRequestReplenished).TotalMilliseconds / ratePeriodMilliseconds); WriteDebug($"{requestsToReplenish} tokens to replenish since {LastRequestReplenished}", isDebug); if (requestsToReplenish == 0 || RequestsUsed == 0) { @@ -118,7 +118,7 @@ private void IncrementAvailableTokens(bool isDebug) } // will try to decrement available tokens, will fail if another thread has used last of burst quota - private bool TryDecrementAvailableTokens(bool isDebug) + private bool TryIncrementRequestsUsed(bool isDebug) { var succeeded = false; diff --git a/Source/Tests/GlobalUsings.cs b/Source/Tests/GlobalUsings.cs new file mode 100644 index 00000000..cefced49 --- /dev/null +++ b/Source/Tests/GlobalUsings.cs @@ -0,0 +1 @@ +global using NUnit.Framework; \ No newline at end of file diff --git a/Source/Tests/RateLimitsTests.cs b/Source/Tests/RateLimitsTests.cs new file mode 100644 index 00000000..0911d1b9 --- /dev/null +++ b/Source/Tests/RateLimitsTests.cs @@ -0,0 +1,40 @@ +using FikaAmazonAPI.Utils; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Tests +{ + [TestFixture] + public class RateLimitsTests + { + [Test] + [TestCase(0.1, 1, 3, 20000)] + [TestCase(0.5, 1, 3, 4000)] + [TestCase(1, 5, 10, 5000)] + public async Task WaitForPermittedRequest_WaitsExpectedLengthOfTime(decimal rate, int burst, int numberRequests, int expectedWaitMilliseconds) + { + // Arrange + var rateLimit = new RateLimits(rate, burst, RateLimitType.UNSET); + + var stopwatch = new Stopwatch(); + var cancellationToken = new CancellationToken(); + + // Act + stopwatch.Start(); + for (int i = 0; i < numberRequests; i++) + { + await rateLimit.WaitForPermittedRequest(cancellationToken, debugMode: true); + } + stopwatch.Stop(); + + // Assert + Assert.That(stopwatch.ElapsedMilliseconds, Is.GreaterThanOrEqualTo(expectedWaitMilliseconds)); + // allow a second for additional time taken by the test to run + Assert.That(stopwatch.ElapsedMilliseconds, Is.LessThanOrEqualTo(expectedWaitMilliseconds + 1000)); + } + } +} diff --git a/Source/Tests/Tests.csproj b/Source/Tests/Tests.csproj new file mode 100644 index 00000000..404e8228 --- /dev/null +++ b/Source/Tests/Tests.csproj @@ -0,0 +1,24 @@ + + + + net8.0 + enable + enable + + false + true + + + + + + + + + + + + + + + From 624d5b0eead02965d53263b2f41456792e7aae3b Mon Sep 17 00:00:00 2001 From: Finn Reilly Date: Thu, 9 Jan 2025 10:53:26 +0000 Subject: [PATCH 2/3] correct sample code program * should intialise multiple threads simultaneously --- Source/FikaAmazonAPI.SampleCode/Program.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/FikaAmazonAPI.SampleCode/Program.cs b/Source/FikaAmazonAPI.SampleCode/Program.cs index 647ed438..447349d7 100644 --- a/Source/FikaAmazonAPI.SampleCode/Program.cs +++ b/Source/FikaAmazonAPI.SampleCode/Program.cs @@ -20,7 +20,7 @@ static async Task Main(string[] args) RefreshToken: config.GetSection("FikaAmazonAPI:RefreshToken").Value, rateLimitingHandler: new RateLimitingHandler()); - var tasks = new[] { 1..80 }.Select(x => + var tasks = Enumerable.Range(1, 10).Select(x => Task.Run(() => { var amazonConnection = connectionFactory.RequestScopedConnection( From 4e59c77f3568d962d2346e868a9aa054341f2f74 Mon Sep 17 00:00:00 2001 From: Finn Reilly Date: Thu, 9 Jan 2025 11:05:24 +0000 Subject: [PATCH 3/3] target .net 6.0 in tests instead --- Source/Tests/Tests.csproj | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Source/Tests/Tests.csproj b/Source/Tests/Tests.csproj index 404e8228..7e999b6d 100644 --- a/Source/Tests/Tests.csproj +++ b/Source/Tests/Tests.csproj @@ -1,7 +1,7 @@ - net8.0 + net6.0 enable enable