Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make RateLimitPartition.Factory public #71145

Merged
merged 3 commits into from
Jun 22, 2022
Merged

Make RateLimitPartition.Factory public #71145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
fb357e6
Make RateLimitPartition.Factory public
wtgodbe Jun 22, 2022
6ef0e91
Fix tests
wtgodbe Jun 22, 2022
6f8c36a
Fix comment
wtgodbe Jun 22, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/libraries/System.Threading.RateLimiting/ref/System.Threading.RateLimiting.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ public partial struct RateLimitPartition<TKey>
private int _dummyPrimitive;
public RateLimitPartition(TKey partitionKey, System.Func<TKey, System.Threading.RateLimiting.RateLimiter> factory) { throw null; }
public readonly TKey PartitionKey { get { throw null; } }
public readonly Func<TKey, RateLimiter> Factory { get { throw null; } }
}
public abstract partial class ReplenishingRateLimiter : System.Threading.RateLimiting.RateLimiter
{
Expand Down
5 changes: 4 additions & 1 deletion ...s/System.Threading.RateLimiting/src/System/Threading/RateLimiting/RateLimitPartition.T.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ public RateLimitPartition(TKey partitionKey, Func<TKey, RateLimiter> factory)
/// </summary>
public TKey PartitionKey { get; }

internal readonly Func<TKey, RateLimiter> Factory;
/// <summary>
/// The function called when a rate limiter for the given partitionKey is needed.
/// </summary>
public Func<TKey, RateLimiter> Factory { get; }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add at least minimal tests that validate the factory supplied to the ctor is roundtripped through this property?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6ef0e91

}
}
27 changes: 7 additions & 20 deletions src/libraries/System.Threading.RateLimiting/tests/RateLimiterPartitionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,7 @@ public void Create_Concurrency()
var options = new ConcurrencyLimiterOptions(10, QueueProcessingOrder.OldestFirst, 10);
var partition = RateLimitPartition.CreateConcurrencyLimiter(1, key => options);

var factoryProperty = typeof(RateLimitPartition<int>).GetField("Factory", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!;
var factory = (Func<int, RateLimiter>)factoryProperty.GetValue(partition);
var limiter = factory(1);
var limiter = partition.Factory(1);
var concurrencyLimiter = Assert.IsType<ConcurrencyLimiter>(limiter);
Assert.Equal(options.PermitLimit, concurrencyLimiter.GetAvailablePermits());
}
Expand All @@ -27,9 +25,7 @@ public void Create_TokenBucket()
var options = new TokenBucketRateLimiterOptions(1, QueueProcessingOrder.OldestFirst, 10, TimeSpan.FromMinutes(1), 1, true);
var partition = RateLimitPartition.CreateTokenBucketLimiter(1, key => options);

var factoryProperty = typeof(RateLimitPartition<int>).GetField("Factory", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!;
var factory = (Func<int, RateLimiter>)factoryProperty.GetValue(partition);
var limiter = factory(1);
var limiter = partition.Factory(1);
var tokenBucketLimiter = Assert.IsType<TokenBucketRateLimiter>(limiter);
Assert.Equal(options.TokenLimit, tokenBucketLimiter.GetAvailablePermits());
Assert.Equal(options.ReplenishmentPeriod, tokenBucketLimiter.ReplenishmentPeriod);
Expand All @@ -41,9 +37,7 @@ public async Task Create_NoLimiter()
{
var partition = RateLimitPartition.CreateNoLimiter(1);

var factoryProperty = typeof(RateLimitPartition<int>).GetField("Factory", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!;
var factory = (Func<int, RateLimiter>)factoryProperty.GetValue(partition);
var limiter = factory(1);
var limiter = partition.Factory(1);

// How do we test an internal implementation of a limiter that doesn't limit? Just try some stuff that normal limiters would probably block on and see if it works.
var available = limiter.GetAvailablePermits();
Expand All @@ -67,15 +61,12 @@ public void Create_AnyLimiter()
{
var partition = RateLimitPartition.Create(1, key => new ConcurrencyLimiter(new ConcurrencyLimiterOptions(1, QueueProcessingOrder.NewestFirst, 10)));

var factoryProperty = typeof(RateLimitPartition<int>).GetField("Factory", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!;
var factory = (Func<int, RateLimiter>)factoryProperty.GetValue(partition);
var limiter = factory(1);
var limiter = partition.Factory(1);
var concurrencyLimiter = Assert.IsType<ConcurrencyLimiter>(limiter);
Assert.Equal(1, concurrencyLimiter.GetAvailablePermits());

var partition2 = RateLimitPartition.Create(1, key => new TokenBucketRateLimiter(new TokenBucketRateLimiterOptions(1, QueueProcessingOrder.NewestFirst, 10, TimeSpan.FromMilliseconds(100), 1, autoReplenishment: false)));
factory = (Func<int, RateLimiter>)factoryProperty.GetValue(partition2);
limiter = factory(1);
limiter = partition2.Factory(1);
var tokenBucketLimiter = Assert.IsType<TokenBucketRateLimiter>(limiter);
Assert.Equal(1, tokenBucketLimiter.GetAvailablePermits());
}
Expand All @@ -86,9 +77,7 @@ public void Create_FixedWindow()
var options = new FixedWindowRateLimiterOptions(1, QueueProcessingOrder.OldestFirst, 10, TimeSpan.FromMinutes(1), true);
var partition = RateLimitPartition.CreateFixedWindowLimiter(1, key => options);

var factoryProperty = typeof(RateLimitPartition<int>).GetField("Factory", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!;
var factory = (Func<int, RateLimiter>)factoryProperty.GetValue(partition);
var limiter = factory(1);
var limiter = partition.Factory(1);
var fixedWindowLimiter = Assert.IsType<FixedWindowRateLimiter>(limiter);
Assert.Equal(options.PermitLimit, fixedWindowLimiter.GetAvailablePermits());
Assert.Equal(options.Window, fixedWindowLimiter.ReplenishmentPeriod);
Expand All @@ -101,9 +90,7 @@ public void Create_SlidingWindow()
var options = new SlidingWindowRateLimiterOptions(1, QueueProcessingOrder.OldestFirst, 10, TimeSpan.FromSeconds(33), 3, true);
var partition = RateLimitPartition.CreateSlidingWindowLimiter(1, key => options);

var factoryProperty = typeof(RateLimitPartition<int>).GetField("Factory", Reflection.BindingFlags.NonPublic | Reflection.BindingFlags.Instance)!;
var factory = (Func<int, RateLimiter>)factoryProperty.GetValue(partition);
var limiter = factory(1);
var limiter = partition.Factory(1);
var slidingWindowLimiter = Assert.IsType<SlidingWindowRateLimiter>(limiter);
Assert.Equal(options.PermitLimit, slidingWindowLimiter.GetAvailablePermits());
Assert.Equal(TimeSpan.FromSeconds(11), slidingWindowLimiter.ReplenishmentPeriod);
Expand Down