Skip to content

Commit 1373e87

Browse files
committed
Feedback again
1 parent e0a277f commit 1373e87

10 files changed

+27
-41
lines changed

src/Middleware/RateLimiting/src/DefaultCombinedLease.cs

+8-14
Original file line numberDiff line numberDiff line change
@@ -7,11 +7,11 @@ namespace Microsoft.AspNetCore.RateLimiting;
77

88
internal sealed class DefaultCombinedLease : RateLimitLease
99
{
10-
private readonly RateLimitLease? _globalLease;
10+
private readonly RateLimitLease _globalLease;
1111
private readonly RateLimitLease _endpointLease;
1212
private HashSet<string>? _metadataNames;
1313

14-
public DefaultCombinedLease(RateLimitLease? globalLease, RateLimitLease endpointLease)
14+
public DefaultCombinedLease(RateLimitLease globalLease, RateLimitLease endpointLease)
1515
{
1616
_globalLease = globalLease;
1717
_endpointLease = endpointLease;
@@ -26,12 +26,9 @@ public override IEnumerable<string> MetadataNames
2626
if (_metadataNames is null)
2727
{
2828
_metadataNames = new HashSet<string>();
29-
if (_globalLease is not null)
29+
foreach (var metadataName in _globalLease.MetadataNames)
3030
{
31-
foreach (var metadataName in _globalLease.MetadataNames)
32-
{
33-
_metadataNames.Add(metadataName);
34-
}
31+
_metadataNames.Add(metadataName);
3532
}
3633
foreach (var metadataName in _endpointLease.MetadataNames)
3734
{
@@ -51,12 +48,9 @@ public override bool TryGetMetadata(string metadataName, out object? metadata)
5148
{
5249
return true;
5350
}
54-
if (_globalLease is not null)
51+
if (_globalLease.TryGetMetadata(metadataName, out metadata))
5552
{
56-
if (_globalLease.TryGetMetadata(metadataName, out metadata))
57-
{
58-
return true;
59-
}
53+
return true;
6054
}
6155

6256
metadata = null;
@@ -82,11 +76,11 @@ protected override void Dispose(bool disposing)
8276

8377
try
8478
{
85-
_globalLease?.Dispose();
79+
_globalLease.Dispose();
8680
}
8781
catch (Exception ex)
8882
{
89-
exceptions ??= new List<Exception>();
83+
exceptions ??= new List<Exception>(1);
9084
exceptions.Add(ex);
9185
}
9286

src/Middleware/RateLimiting/src/DefaultKeyType.cs

+5-1
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,17 @@ namespace Microsoft.AspNetCore.RateLimiting;
55

66
internal struct DefaultKeyType
77
{
8-
public DefaultKeyType(string policyName, object? key)
8+
public DefaultKeyType(string policyName, object? key, object? factory = null)
99
{
1010
PolicyName = policyName;
1111
Key = key;
12+
Factory = factory;
1213
}
1314

1415
public string PolicyName { get; }
1516

1617
public object? Key { get; }
18+
19+
// This is really a Func<TPartitionKey, RateLimiter>
20+
public object? Factory { get; }
1721
}

src/Middleware/RateLimiting/src/DefaultKeyTypeEqualityComparer.cs

+2-1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Diagnostics.CodeAnalysis;
55

66
namespace Microsoft.AspNetCore.RateLimiting;
7+
78
internal class DefaultKeyTypeEqualityComparer : IEqualityComparer<DefaultKeyType>
89
{
910
public bool Equals(DefaultKeyType x, DefaultKeyType y)
@@ -12,7 +13,7 @@ public bool Equals(DefaultKeyType x, DefaultKeyType y)
1213
var yKey = y.Key;
1314
if (xKey == null && yKey == null)
1415
{
15-
return true;
16+
return x.PolicyName.Equals(y.PolicyName);
1617
}
1718
else if (xKey == null || yKey == null)
1819
{

src/Middleware/RateLimiting/src/DefaultRateLimiterPolicy.cs

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
using Microsoft.AspNetCore.Http;
66

77
namespace Microsoft.AspNetCore.RateLimiting;
8+
89
internal sealed class DefaultRateLimiterPolicy : IRateLimiterPolicy<DefaultKeyType>
910
{
1011
private readonly Func<HttpContext, RateLimitPartition<DefaultKeyType>> _partitioner;

src/Middleware/RateLimiting/src/LeaseContext.cs

+1
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
using System.Threading.RateLimiting;
55

66
namespace Microsoft.AspNetCore.RateLimiting;
7+
78
internal struct LeaseContext : IDisposable
89
{
910
public bool? GlobalRejected { get; init; }

src/Middleware/RateLimiting/src/PolicyNameKey.cs

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
namespace Microsoft.AspNetCore.RateLimiting;
5+
56
internal sealed class PolicyNameKey
67
{
78
public required string PolicyName { get; init; }

src/Middleware/RateLimiting/src/PolicyTypeInfo.cs

-12
This file was deleted.

src/Middleware/RateLimiting/src/RateLimiterMetadata.cs

+3-3
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,16 @@ namespace Microsoft.AspNetCore.RateLimiting;
99
internal class RateLimiterMetadata : IRateLimiterMetadata
1010
{
1111
/// <summary>
12-
/// Creates a new instance of <see cref="RateLimiterMetadata"/> using the specified limiter.
12+
/// Creates a new instance of <see cref="RateLimiterMetadata"/> using the specified policy.
1313
/// </summary>
14-
/// <param name="policyName">The name of the limiter which needs to be applied.</param>
14+
/// <param name="policyName">The name of the policy which needs to be applied.</param>
1515
public RateLimiterMetadata(string policyName)
1616
{
1717
PolicyName = policyName;
1818
}
1919

2020
/// <summary>
21-
/// The name of the limiter which needs to be applied.
21+
/// The name of the policy which needs to be applied.
2222
/// </summary>
2323
public string PolicyName { get; }
2424
}

src/Middleware/RateLimiting/src/RateLimiterOptions.cs

+4-7
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,10 @@ namespace Microsoft.AspNetCore.RateLimiting;
1313
/// </summary>
1414
public sealed class RateLimiterOptions
1515
{
16-
// Stores all of the keys for each partition so that we reuse the same objects.
17-
internal IDictionary<string, ISet<DefaultKeyType>> _partitionKeys = new Dictionary<string, ISet<DefaultKeyType>>();
18-
19-
internal IDictionary<string, DefaultRateLimiterPolicy> PolicyMap { get; }
16+
internal Dictionary<string, DefaultRateLimiterPolicy> PolicyMap { get; }
2017
= new Dictionary<string, DefaultRateLimiterPolicy>(StringComparer.Ordinal);
2118

22-
internal IDictionary<string, Func<IServiceProvider, DefaultRateLimiterPolicy>> UnactivatedPolicyMap { get; }
19+
internal Dictionary<string, Func<IServiceProvider, DefaultRateLimiterPolicy>> UnactivatedPolicyMap { get; }
2320
= new Dictionary<string, Func<IServiceProvider, DefaultRateLimiterPolicy>>(StringComparer.Ordinal);
2421

2522
/// <summary>
@@ -115,8 +112,8 @@ private static Func<HttpContext, RateLimitPartition<DefaultKeyType>> ConvertPart
115112
return (context =>
116113
{
117114
RateLimitPartition<TPartitionKey> partition = partitioner(context);
118-
var partitionKey = new DefaultKeyType(policyName, partition.PartitionKey);
119-
return new RateLimitPartition<DefaultKeyType>(partitionKey, key => partition.Factory(partition.PartitionKey));
115+
var partitionKey = new DefaultKeyType(policyName, partition.PartitionKey, partition.Factory);
116+
return new RateLimitPartition<DefaultKeyType>(partitionKey, key => ((Func<TPartitionKey, RateLimiter>)partitionKey.Factory!)((TPartitionKey)partitionKey.Key!));
120117
});
121118
}
122119

src/Middleware/RateLimiting/src/RateLimitingMiddleware.cs

+2-3
Original file line numberDiff line numberDiff line change
@@ -130,8 +130,7 @@ private LeaseContext CombinedAcquire(HttpContext context)
130130
globalLease?.Dispose();
131131
throw;
132132
}
133-
134-
return new LeaseContext() { Lease = new DefaultCombinedLease(globalLease, endpointLease)};
133+
return globalLease is null ? new LeaseContext() { Lease = endpointLease } : new LeaseContext() { Lease = new DefaultCombinedLease(globalLease, endpointLease) };
135134
}
136135

137136
private async ValueTask<LeaseContext> CombinedWaitAsync(HttpContext context, CancellationToken cancellationToken)
@@ -163,7 +162,7 @@ private async ValueTask<LeaseContext> CombinedWaitAsync(HttpContext context, Can
163162
throw;
164163
}
165164

166-
return new LeaseContext() { Lease = new DefaultCombinedLease(globalLease, endpointLease) };
165+
return globalLease is null ? new LeaseContext() { Lease = endpointLease } : new LeaseContext() { Lease = new DefaultCombinedLease(globalLease, endpointLease) };
167166
}
168167

169168
// Create the endpoint-specific PartitionedRateLimiter

0 commit comments

Comments
 (0)