Merge pull request #816 from FinnReilly/feature/addTestsToRateLimitingCodeAndResolveAnyBugs

abuzuhri · web-flow · commit b2eabc49d0e1 · 2025-01-19T18:41:43.000+01:00
Feature/add tests to rate limiting code and resolve any bugs
diff --git 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,13 +34,18 @@ 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
 	EndGlobalSection
 	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
@@ -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 = Enumerable.Range(1, 10).Select(x =>
             Task.Run(() =>
             {
                 var amazonConnection = connectionFactory.RequestScopedConnection(
diff --git 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
@@ -0,0 +1 @@
+global using NUnit.Framework;
diff --git a/Source/Tests/RateLimitsTests.cs 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
@@ -0,0 +1,24 @@
+<Project Sdk="Microsoft.NET.Sdk">
+
+  <PropertyGroup>
+    <TargetFramework>net6.0</TargetFramework>
+    <ImplicitUsings>enable</ImplicitUsings>
+    <Nullable>enable</Nullable>
+
+    <IsPackable>false</IsPackable>
+    <IsTestProject>true</IsTestProject>
+  </PropertyGroup>
+
+  <ItemGroup>
+    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.0" />
+    <PackageReference Include="NUnit" Version="3.13.3" />
+    <PackageReference Include="NUnit3TestAdapter" Version="4.2.1" />
+    <PackageReference Include="NUnit.Analyzers" Version="3.6.1" />
+    <PackageReference Include="coverlet.collector" Version="6.0.0" />
+  </ItemGroup>
+
+  <ItemGroup>
+    <ProjectReference Include="..\FikaAmazonAPI\FikaAmazonAPI.csproj" />
+  </ItemGroup>
+
+</Project>

Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ static async Task Main(string[] args)`
`20`	`20`	`RefreshToken: config.GetSection("FikaAmazonAPI:RefreshToken").Value,`
`21`	`21`	`rateLimitingHandler: new RateLimitingHandler());`
`22`	`22`
`23`		`- var tasks = new[] { 1..10 }.Select(x =>`
	`23`	`+ var tasks = Enumerable.Range(1, 10).Select(x =>`
`24`	`24`	`Task.Run(() =>`
`25`	`25`	`{`
`26`	`26`	`var amazonConnection = connectionFactory.RequestScopedConnection(`
Original file line number	Diff line number	Diff line change
`@@ -1,8 +1,9 @@`
`1`	`1`	`using System;`
`2`		`-using System.Runtime;`
	`2`	`+using System.Runtime.CompilerServices;`
`3`	`3`	`using System.Threading;`
`4`	`4`	`using System.Threading.Tasks;`
`5`	`5`
	`6`	`+[assembly: InternalsVisibleTo("Tests")]`
`6`	`7`	`namespace FikaAmazonAPI.Utils`
`7`	`8`	`{`
`8`	`9`	`internal class RateLimits`
`@@ -47,10 +48,9 @@ public async Task WaitForPermittedRequest(CancellationToken cancellationToken =`
`47`	`48`	`}`
`48`	`49`
`49`	`50`	`int ratePeriodMs = GetRatePeriodMs();`
`50`		`- var requestsUsedAtOnset = RequestsUsed;`
`51`	`51`
`52`	`52`	`// when requests used more than zero, replenish according to time elapsed`
`53`		`- IncrementAvailableTokens(debugMode);`
	`53`	`+ DecrementRequestsUsed(debugMode);`
`54`	`54`
`55`	`55`	`var nextRequestsSent = RequestsUsed + 1;`
`56`	`56`	`var nextRequestsSentTxt = (nextRequestsSent > Burst) ? "FULL" : nextRequestsSent.ToString();`
`@@ -76,7 +76,7 @@ public async Task WaitForPermittedRequest(CancellationToken cancellationToken =`
`76`	`76`	`else`
`77`	`77`	`{`
`78`	`78`	`// replenish token`
`79`		`- IncrementAvailableTokens(debugMode);`
	`79`	`+ DecrementRequestsUsed(debugMode);`
`80`	`80`	`}`
`81`	`81`
`82`	`82`	`if (RequestsUsed <= 0)`
`@@ -87,7 +87,7 @@ public async Task WaitForPermittedRequest(CancellationToken cancellationToken =`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`// now remove token from bucket for pending request`
`90`		`- requestIsPermitted = TryDecrementAvailableTokens(debugMode);`
	`90`	`+ requestIsPermitted = TryIncrementRequestsUsed(debugMode);`
`91`	`91`	`}`
`92`	`92`	`}`
`93`	`93`
`@@ -97,13 +97,13 @@ internal void SetRateLimit(decimal rate)`
`97`	`97`	`}`
`98`	`98`
`99`	`99`	`// increments available tokens, unless another thread has already incremented them.`
`100`		`- private void IncrementAvailableTokens(bool isDebug)`
	`100`	`+ private void DecrementRequestsUsed(bool isDebug)`
`101`	`101`	`{`
`102`	`102`	`WriteDebug($"Attempting to increment tokens", isDebug);`
`103`	`103`	`lock (_locker)`
`104`	`104`	`{`
`105`	`105`	`var ratePeriodMilliseconds = GetRatePeriodMs();`
`106`		`- var requestsToReplenish = ratePeriodMilliseconds == 0 ? 0 : (DateTime.UtcNow - LastRequestReplenished).Milliseconds / ratePeriodMilliseconds;`
	`106`	`+ var requestsToReplenish = ratePeriodMilliseconds == 0 ? 0 : (int)((DateTime.UtcNow - LastRequestReplenished).TotalMilliseconds / ratePeriodMilliseconds);`
`107`	`107`	`WriteDebug($"{requestsToReplenish} tokens to replenish since {LastRequestReplenished}", isDebug);`
`108`	`108`	`if (requestsToReplenish == 0 \|\| RequestsUsed == 0)`
`109`	`109`	`{`
`@@ -118,7 +118,7 @@ private void IncrementAvailableTokens(bool isDebug)`
`118`	`118`	`}`
`119`	`119`
`120`	`120`	`// will try to decrement available tokens, will fail if another thread has used last of burst quota`
`121`		`- private bool TryDecrementAvailableTokens(bool isDebug)`
	`121`	`+ private bool TryIncrementRequestsUsed(bool isDebug)`
`122`	`122`	`{`
`123`	`123`	`var succeeded = false;`
`124`	`124`