Skip to content

Commit 8824cee

Browse files
yjorayevyjorayev
committed
addressed PR comments
1 parent 617c85a commit 8824cee

19 files changed

+67
-142
lines changed

Diff for: ‎samples/RateLimiter/Ocelot.Samples.RateLimiter.http

+4-2
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
11
@RateLimiterSample_HostAddress = http://localhost:5202
22

3-
GET {{RateLimiterSample_HostAddress}}/laura/
3+
GET {{RateLimiterSample_HostAddress}}/laura
44
Accept: application/json
55

6-
GET {{RateLimiterSample_HostAddress}}/tom/
6+
###
7+
8+
GET {{RateLimiterSample_HostAddress}}/tom
79
Accept: application/json
810

911
###

Diff for: ‎samples/RateLimiter/ocelot.json

+1-2
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,7 @@
2727
"Key": "Tom",
2828
"RateLimitOptions": {
2929
"EnableRateLimiting": true,
30-
"RateLimitMiddlewareType": "DotNet",
31-
"RateLimitPolicyName": "fixed"
30+
"Policy": "fixed"
3231
}
3332
}
3433
],

Diff for: ‎src/Ocelot/Configuration/Builder/RateLimitOptionsBuilder.cs

+1-8
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ public class RateLimitOptionsBuilder
1010
private string _rateLimitCounterPrefix;
1111
private RateLimitRule _rateLimitRule;
1212
private int _httpStatusCode;
13-
private RateLimitMiddlewareType _rateLimitMiddlewareType;
1413
private string _rateLimitPolicyName;
1514

1615
public RateLimitOptionsBuilder WithEnableRateLimiting(bool enableRateLimiting)
@@ -61,12 +60,6 @@ public RateLimitOptionsBuilder WithHttpStatusCode(int httpStatusCode)
6160
return this;
6261
}
6362

64-
public RateLimitOptionsBuilder WithRateLimitMiddlewareType(RateLimitMiddlewareType middlewareType)
65-
{
66-
_rateLimitMiddlewareType = middlewareType;
67-
return this;
68-
}
69-
7063
public RateLimitOptionsBuilder WithRateLimitPolicyName(string policyName)
7164
{
7265
_rateLimitPolicyName = policyName;
@@ -77,7 +70,7 @@ public RateLimitOptions Build()
7770
{
7871
return new RateLimitOptions(_enableRateLimiting, _clientIdHeader, _getClientWhitelist,
7972
_disableRateLimitHeaders, _quotaExceededMessage, _rateLimitCounterPrefix,
80-
_rateLimitRule, _httpStatusCode, _rateLimitMiddlewareType, _rateLimitPolicyName);
73+
_rateLimitRule, _httpStatusCode, _rateLimitPolicyName);
8174
}
8275
}
8376
}

Diff for: ‎src/Ocelot/Configuration/Creator/RateLimitOptionsCreator.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,7 @@ public RateLimitOptions Create(FileRateLimitRule fileRateLimitRule, FileGlobalCo
2020
.WithRateLimitRule(new RateLimitRule(fileRateLimitRule.Period,
2121
fileRateLimitRule.PeriodTimespan,
2222
fileRateLimitRule.Limit))
23-
.WithRateLimitMiddlewareType(fileRateLimitRule.RateLimitMiddlewareType)
24-
.WithRateLimitPolicyName(fileRateLimitRule.RateLimitPolicyName)
23+
.WithRateLimitPolicyName(fileRateLimitRule.Policy)
2524
.Build();
2625
}
2726

Diff for: ‎src/Ocelot/Configuration/File/FileRateLimitRule.cs

+5-16
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ public FileRateLimitRule(FileRateLimitRule from)
1414
Limit = from.Limit;
1515
Period = from.Period;
1616
PeriodTimespan = from.PeriodTimespan;
17-
RateLimitMiddlewareType = from.RateLimitMiddlewareType;
18-
RateLimitPolicyName = from.RateLimitPolicyName;
17+
Policy = from.Policy;
1918
}
2019

2120
/// <summary>
@@ -58,21 +57,13 @@ public FileRateLimitRule(FileRateLimitRule from)
5857
/// </value>
5958
public long Limit { get; set; }
6059

61-
/// <summary>
62-
/// Rate limit middleware type.
63-
/// </summary>
64-
/// <value>
65-
/// <see cref="RateLimitMiddlewareType.Ocelot" />, <see cref="RateLimitMiddlewareType.DotNet" />
66-
/// </value>
67-
public RateLimitMiddlewareType RateLimitMiddlewareType { get; set; }
68-
6960
/// <summary>
7061
/// Rate limit policy name. It only takes effect if rate limit middleware type is set to DotNet.
7162
/// </summary>
7263
/// <value>
7364
/// A string of rate limit policy name.
7465
/// </value>
75-
public string RateLimitPolicyName { get; set; }
66+
public string Policy { get; set; }
7667

7768
/// <inheritdoc/>
7869
public override string ToString()
@@ -84,13 +75,11 @@ public override string ToString()
8475

8576
var sb = new StringBuilder();
8677

87-
sb.Append($"{nameof(RateLimitMiddlewareType)}:{RateLimitMiddlewareType},");
88-
89-
if (RateLimitMiddlewareType == RateLimitMiddlewareType.DotNet)
78+
if (!string.IsNullOrWhiteSpace(Policy))
9079
{
91-
sb.Append($"{nameof(RateLimitPolicyName)}:{RateLimitPolicyName}");
80+
sb.Append($"{nameof(Policy)}:{Policy}");
9281
}
93-
else if (RateLimitMiddlewareType == RateLimitMiddlewareType.Ocelot)
82+
else
9483
{
9584
sb.Append(
9685
$"{nameof(Period)}:{Period},{nameof(PeriodTimespan)}:{PeriodTimespan:F},{nameof(Limit)}:{Limit},{nameof(ClientWhitelist)}:[");

Diff for: ‎src/Ocelot/Configuration/RateLimitMiddlewareType.cs

-7
This file was deleted.

Diff for: ‎src/Ocelot/Configuration/RateLimitOptions.cs

+3-7
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,7 @@ public class RateLimitOptions
88
private readonly Func<List<string>> _getClientWhitelist;
99

1010
public RateLimitOptions(bool enableRateLimiting, string clientIdHeader, Func<List<string>> getClientWhitelist, bool disableRateLimitHeaders,
11-
string quotaExceededMessage, string rateLimitCounterPrefix, RateLimitRule rateLimitRule, int httpStatusCode,
12-
RateLimitMiddlewareType rateLimitMiddlewareType, string rateLimitPolicyName)
11+
string quotaExceededMessage, string rateLimitCounterPrefix, RateLimitRule rateLimitRule, int httpStatusCode, string rateLimitPolicy = null)
1312
{
1413
EnableRateLimiting = enableRateLimiting;
1514
ClientIdHeader = clientIdHeader;
@@ -19,8 +18,7 @@ public RateLimitOptions(bool enableRateLimiting, string clientIdHeader, Func<Lis
1918
RateLimitCounterPrefix = rateLimitCounterPrefix;
2019
RateLimitRule = rateLimitRule;
2120
HttpStatusCode = httpStatusCode;
22-
RateLimitMiddlewareType = rateLimitMiddlewareType;
23-
RateLimiterPolicyName = rateLimitPolicyName;
21+
Policy = rateLimitPolicy;
2422
}
2523

2624
/// <summary>
@@ -90,8 +88,6 @@ public RateLimitOptions(bool enableRateLimiting, string clientIdHeader, Func<Lis
9088
/// </value>
9189
public bool DisableRateLimitHeaders { get; }
9290

93-
public RateLimitMiddlewareType RateLimitMiddlewareType { get; }
94-
95-
public string RateLimiterPolicyName { get; }
91+
public string Policy { get; }
9692
}
9793
}

Diff for: ‎src/Ocelot/Configuration/Validator/RouteFluentValidator.cs

+1-12
Original file line numberDiff line numberDiff line change
@@ -66,12 +66,6 @@ public RouteFluentValidator(IAuthenticationSchemeProvider authenticationSchemePr
6666
.Must(IsValidPeriod)
6767
.WithMessage("RateLimitOptions.Period does not contain integer then s (second), m (minute), h (hour), d (day) e.g. 1m for 1 minute period");
6868
});
69-
70-
When(IsDotNetRateLimiter, () => {
71-
RuleFor(route => route.RateLimitOptions.RateLimitPolicyName)
72-
.NotEmpty()
73-
.WithMessage("RateLimitOptions.RateLimitPolicyName is required when RateLimitOptions.RateLimitMiddlewareType is DotNet.");
74-
});
7569
});
7670

7771
RuleFor(route => route.AuthenticationOptions)
@@ -118,12 +112,7 @@ private async Task<bool> IsSupportedAuthenticationProviders(FileAuthenticationOp
118112

119113
private static bool IsOcelotRateLimiter(FileRoute fileRoute)
120114
{
121-
return fileRoute.RateLimitOptions.RateLimitMiddlewareType == RateLimitMiddlewareType.Ocelot;
122-
}
123-
124-
private static bool IsDotNetRateLimiter(FileRoute fileRoute)
125-
{
126-
return fileRoute.RateLimitOptions.RateLimitMiddlewareType == RateLimitMiddlewareType.DotNet;
115+
return string.IsNullOrWhiteSpace(fileRoute.RateLimitOptions.Policy);
127116
}
128117

129118
private static bool IsValidPeriod(FileRateLimitRule rateLimitOptions)

Diff for: ‎src/Ocelot/DependencyInjection/Features.cs

+17-7
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,22 @@ public static class Features
2626
/// <param name="services">The services collection to add the feature to.</param>
2727
/// <param name="configurationRoot">Root configuration object.</param>
2828
/// <returns>The same <see cref="IServiceCollection"/> object.</returns>
29-
public static IServiceCollection AddRateLimiting(this IServiceCollection services, IConfiguration configurationRoot)
30-
{
31-
services
32-
.AddSingleton<IRateLimiting, RateLimiting.RateLimiting>()
33-
.AddSingleton<IRateLimitStorage, MemoryCacheRateLimitStorage>();
34-
29+
public static IServiceCollection AddRateLimiting(this IServiceCollection services) => services
30+
.AddSingleton<IRateLimiting, RateLimiting.RateLimiting>()
31+
.AddSingleton<IRateLimitStorage, MemoryCacheRateLimitStorage>();
32+
3533
#if NET7_0_OR_GREATER
34+
/// <summary>
35+
/// Ocelot feature: <see href="">AspNet Rate Limiting</see>.
36+
/// </summary>
37+
/// <remarks>
38+
/// Read The Docs: <see href="">Rate Limiting</see>.
39+
/// </remarks>
40+
/// <param name="services">The services collection to add the feature to.</param>
41+
/// <param name="configurationRoot">Root configuration object.</param>
42+
/// <returns>The same <see cref="IServiceCollection"/> object.</returns>
43+
public static IServiceCollection AddAspNetRateLimiting(this IServiceCollection services, IConfiguration configurationRoot)
44+
{
3645
var globalRateLimitOptions = configurationRoot.Get<FileConfiguration>()?.GlobalConfiguration?.RateLimitOptions;
3746
var rejectStatusCode = globalRateLimitOptions?.HttpStatusCode ?? StatusCodes.Status429TooManyRequests;
3847
var rejectedMessage = globalRateLimitOptions?.QuotaExceededMessage ?? "API calls quota exceeded!";
@@ -44,9 +53,10 @@ public static IServiceCollection AddRateLimiting(this IServiceCollection service
4453
await rejectedContext.HttpContext.Response.WriteAsync(rejectedMessage, token);
4554
};
4655
});
47-
#endif
56+
4857
return services;
4958
}
59+
#endif
5060

5161
/// <summary>
5262
/// Ocelot feature: <see href="https://github.com/ThreeMammals/Ocelot/blob/develop/docs/features/caching.rst">Request Caching</see>.

Diff for: ‎src/Ocelot/DependencyInjection/OcelotBuilder.cs

+4-1
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,10 @@ public OcelotBuilder(IServiceCollection services, IConfiguration configurationRo
103103
Services.TryAddSingleton<IDownstreamRouteProviderFactory, DownstreamRouteProviderFactory>();
104104
Services.TryAddSingleton<IHttpResponder, HttpContextResponder>();
105105
Services.TryAddSingleton<IErrorsToHttpStatusCodeMapper, ErrorsToHttpStatusCodeMapper>();
106-
Services.AddRateLimiting(configurationRoot); // Feature: Rate Limiting
106+
Services.AddRateLimiting(); // Feature: Rate Limiting
107+
#if NET7_0_OR_GREATER
108+
Services.AddAspNetRateLimiting(configurationRoot); // Feature: AspNet Rate Limiting
109+
#endif
107110
Services.TryAddSingleton<IRequestMapper, RequestMapper>();
108111
Services.TryAddSingleton<IHttpHandlerOptionsCreator, HttpHandlerOptionsCreator>();
109112
Services.TryAddSingleton<IDownstreamAddressesCreator, DownstreamAddressesCreator>();

Diff for: ‎src/Ocelot/Middleware/OcelotPipelineExtensions.cs

-16
Original file line numberDiff line numberDiff line change
@@ -79,14 +79,6 @@ public static RequestDelegate BuildOcelotPipeline(this IApplicationBuilder app,
7979
// We check whether the request is ratelimit, and if there is no continue processing
8080
app.UseRateLimiting();
8181

82-
//use .Net rate limiter
83-
#if NET7_0_OR_GREATER
84-
app.UseWhen(UseDotNetRateLimiter, rateLimitedApp =>
85-
{
86-
rateLimitedApp.UseRateLimiter();
87-
});
88-
#endif
89-
9082
// This adds or updates the request id (initally we try and set this based on global config in the error handling middleware)
9183
// If anything was set at global level and we have a different setting at re route level the global stuff will be overwritten
9284
// This means you can get a scenario where you have a different request id from the first piece of middleware to the request id middleware.
@@ -136,14 +128,6 @@ public static RequestDelegate BuildOcelotPipeline(this IApplicationBuilder app,
136128
return app.Build();
137129
}
138130

139-
private static bool UseDotNetRateLimiter(HttpContext httpContext)
140-
{
141-
var downstreamRoute = httpContext.Items.DownstreamRoute();
142-
143-
return downstreamRoute?.RateLimitOptions?.RateLimitMiddlewareType ==
144-
Configuration.RateLimitMiddlewareType.DotNet;
145-
}
146-
147131
private static IApplicationBuilder UseIfNotNull(this IApplicationBuilder builder, Func<HttpContext, Func<Task>, Task> middleware)
148132
=> middleware != null ? builder.Use(middleware) : builder;
149133

Diff for: ‎src/Ocelot/RateLimiting/Middleware/RateLimitingMiddleware.cs

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,10 +39,10 @@ public async Task Invoke(HttpContext httpContext)
3939
}
4040

4141
#if NET7_0_OR_GREATER
42-
if (options.RateLimitMiddlewareType == RateLimitMiddlewareType.DotNet)
42+
if (!string.IsNullOrWhiteSpace(options.Policy))
4343
{
4444
//add EnableRateLimiting attribute to endpoint, so that .Net rate limiter can pick it up and do its thing
45-
var metadata = new EndpointMetadataCollection(new EnableRateLimitingAttribute(options.RateLimiterPolicyName));
45+
var metadata = new EndpointMetadataCollection(new EnableRateLimitingAttribute(options.Policy));
4646
var endpoint = new Endpoint(null, metadata, "tempEndpoint");
4747
httpContext.SetEndpoint(endpoint);
4848
await _next.Invoke(httpContext);

Diff for: ‎src/Ocelot/RateLimiting/Middleware/RateLimitingMiddlewareExtensions.cs

+19-1
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,29 @@
11
using Microsoft.AspNetCore.Builder;
2+
using Microsoft.AspNetCore.Http;
3+
using Ocelot.Middleware;
24

35
namespace Ocelot.RateLimiting.Middleware;
46

57
public static class RateLimitingMiddlewareExtensions
68
{
79
public static IApplicationBuilder UseRateLimiting(this IApplicationBuilder builder)
810
{
9-
return builder.UseMiddleware<RateLimitingMiddleware>();
11+
builder.UseMiddleware<RateLimitingMiddleware>();
12+
13+
//use AspNet rate limiter
14+
#if NET7_0_OR_GREATER
15+
builder.UseWhen(UseAspNetRateLimiter, rateLimitedApp =>
16+
{
17+
rateLimitedApp.UseRateLimiter();
18+
});
19+
#endif
20+
21+
return builder;
22+
}
23+
24+
private static bool UseAspNetRateLimiter(HttpContext httpContext)
25+
{
26+
var downstreamRoute = httpContext.Items.DownstreamRoute();
27+
return !string.IsNullOrWhiteSpace(downstreamRoute?.RateLimitOptions?.Policy);
1028
}
1129
}

Diff for: ‎test/Ocelot.AcceptanceTests/RateLimiting/RateLimitingTests.cs

+1-2
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,7 @@ public void Should_RateLimit()
6262
RateLimitOptions = new FileRateLimitRule()
6363
{
6464
EnableRateLimiting = true,
65-
RateLimitMiddlewareType = RateLimitMiddlewareType.DotNet,
66-
RateLimitPolicyName = rateLimitPolicyName,
65+
Policy = rateLimitPolicyName,
6766
},
6867
};
6968

Diff for: ‎test/Ocelot.UnitTests/Configuration/DownstreamRouteExtensionsTests.cs

+1-1
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ public DownstreamRouteExtensionsTests()
3131
default,
3232
new CacheOptions(0, null, null, null),
3333
new LoadBalancerOptions(null, null, 0),
34-
new RateLimitOptions(false, null, null, false, null, null, null, 0, RateLimitMiddlewareType.Ocelot, null),
34+
new RateLimitOptions(false, null, null, false, null, null, null, 0),
3535
new Dictionary<string, string>(),
3636
new List<ClaimToThing>(),
3737
new List<ClaimToThing>(),

Diff for: ‎test/Ocelot.UnitTests/Configuration/RateLimitOptionsCreatorTests.cs

+3-5
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,6 @@ public void should_create_rate_limit_options_ocelot()
5454
.WithRateLimitRule(new RateLimitRule(fileRoute.RateLimitOptions.Period,
5555
fileRoute.RateLimitOptions.PeriodTimespan,
5656
fileRoute.RateLimitOptions.Limit))
57-
.WithRateLimitMiddlewareType(RateLimitMiddlewareType.Ocelot)
5857
.Build();
5958

6059
_enabled = false;
@@ -68,14 +67,14 @@ public void should_create_rate_limit_options_ocelot()
6867
}
6968

7069
[Fact]
71-
public void should_create_rate_limit_options_dotnet()
70+
[Trait("Feat", "2138")]
71+
public void should_create_rate_limit_options_aspnet()
7272
{
7373
var fileRoute = new FileRoute
7474
{
7575
RateLimitOptions = new FileRateLimitRule
7676
{
77-
RateLimitMiddlewareType = RateLimitMiddlewareType.DotNet,
78-
RateLimitPolicyName = "test",
77+
Policy = "test",
7978
EnableRateLimiting = true,
8079
},
8180
};
@@ -98,7 +97,6 @@ public void should_create_rate_limit_options_dotnet()
9897
.WithQuotaExceededMessage("QuotaExceededMessage")
9998
.WithRateLimitCounterPrefix("ocelot")
10099
.WithRateLimitRule(new RateLimitRule(null, 0, 0))
101-
.WithRateLimitMiddlewareType(RateLimitMiddlewareType.DotNet)
102100
.WithRateLimitPolicyName("test")
103101
.Build();
104102

0 commit comments

Comments
 (0)