-
Notifications
You must be signed in to change notification settings - Fork 144
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.
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
Requester and RateLimitRequester refactor #481
Open
frederickrogan
wants to merge
14
commits into
BenFradet:develop
Choose a base branch
from
frederickrogan:refactor-requesters
base: develop
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
fd0d7f0
Alternative requester
frederickrogan cf659d9
Split StatusRiotApi
frederickrogan 7d5b526
Implement other http methods in requester
frederickrogan b7c02d0
Refactor rate limited refresher
frederickrogan 902d363
Move riot api to new requester
frederickrogan 39c58ba
Refactor summoner to new requester
frederickrogan 5b62daa
Refactor tournament api to new requester
frederickrogan 8c9c5f8
Refactor static api to new requester
frederickrogan ac43d90
Fix status api for alt requester rename
frederickrogan ce0ee0e
Remove old limited rate requester interface
frederickrogan 4338c6c
Tidy up requester
frederickrogan e136da8
Update core DI setup
frederickrogan 099e3af
Fix status api test dependency on requester
frederickrogan 404bce8
Run async deserialization in task
frederickrogan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,8 @@ | |
using RiotSharp.Interfaces; | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Net.Http; | ||
using Microsoft.Extensions.DependencyInjection.Extensions; | ||
|
||
namespace RiotSharp.AspNetCore | ||
{ | ||
|
@@ -25,18 +27,27 @@ public static IServiceCollection AddRiotSharp(this IServiceCollection serviceCol | |
if (riotSharpOptions.TournamentApi.ApiKey == null && riotSharpOptions.RiotApi.ApiKey == null) | ||
throw new ArgumentNullException("No api key provided.", innerException: null); | ||
|
||
var serializer = new RequestContentSerializer(); | ||
var deserializer = new ResponseDeserializer(); | ||
var httpClient = new HttpClient(); | ||
var failedRequestHandler = new FailedRequestHandler(); | ||
var client = new RequestClient(httpClient, failedRequestHandler); | ||
|
||
if (riotSharpOptions.RiotApi.ApiKey != null) | ||
{ | ||
var rateLimitedRequester = new RateLimitedRequester(riotSharpOptions.RiotApi.ApiKey, | ||
riotSharpOptions.RiotApi.RateLimits); | ||
serviceCollection.AddSingleton<ITournamentRiotApi>(serviceProvider => | ||
new TournamentRiotApi(rateLimitedRequester)); | ||
serviceCollection.AddSingleton<IRiotApi>(serviceProvider => new RiotApi(rateLimitedRequester)); | ||
var requestCreator = new RequestCreator(riotSharpOptions.RiotApi.ApiKey, serializer); | ||
var basicRequester = new Requester(client, requestCreator, deserializer); | ||
var riotRateLimitProvider = new RateLimitProvider(riotSharpOptions.RiotApi.RateLimits); | ||
|
||
|
||
var riotRateLimitedRequester = new RateLimitedRequester(basicRequester, riotRateLimitProvider); | ||
|
||
serviceCollection.AddSingleton<IRiotApi>(provider => new RiotApi(riotRateLimitedRequester)); | ||
|
||
var staticApiRequester = new RateLimitedRequester(riotSharpOptions.RiotApi.ApiKey, new Dictionary<TimeSpan, int> | ||
{ | ||
{ new TimeSpan(1, 0, 0), 10 } | ||
}); | ||
|
||
var staticRateLimitProvider = new RateLimitProvider(new Dictionary<TimeSpan, int>{{new TimeSpan(1, 0, 0), 10}}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. could you limit the lines to 120 chars? |
||
|
||
var staticRateLimtedRequester = new RateLimitedRequester(basicRequester, staticRateLimitProvider); | ||
|
||
if (riotSharpOptions.RiotApi.UseMemoryCache) | ||
{ | ||
|
@@ -49,15 +60,19 @@ public static IServiceCollection AddRiotSharp(this IServiceCollection serviceCol | |
serviceCollection.AddSingleton<ICache, PassThroughCache>(); | ||
|
||
serviceCollection.AddSingleton<IStaticRiotApi>(serviceProvider => | ||
new StaticRiotApi(staticApiRequester, serviceProvider.GetRequiredService<ICache>(), riotSharpOptions.RiotApi.SlidingExpirationTime)); | ||
new StaticRiotApi(staticRateLimtedRequester, serviceProvider.GetRequiredService<ICache>(), riotSharpOptions.RiotApi.SlidingExpirationTime)); | ||
} | ||
|
||
if (riotSharpOptions.TournamentApi.ApiKey != null) | ||
{ | ||
var rateLimitedRequester = new RateLimitedRequester(riotSharpOptions.TournamentApi.ApiKey, | ||
riotSharpOptions.TournamentApi.RateLimits); | ||
serviceCollection.AddSingleton<ITournamentRiotApi>(serviceProvider => | ||
new TournamentRiotApi(rateLimitedRequester, riotSharpOptions.TournamentApi.UseStub)); | ||
var requestCreator = new RequestCreator(riotSharpOptions.TournamentApi.ApiKey, serializer); | ||
var basicRequester = new Requester(client, requestCreator, deserializer); | ||
var riotRateLimitProvider = new RateLimitProvider(riotSharpOptions.TournamentApi.RateLimits); | ||
|
||
|
||
var tournamentRateLimitedRequester = new RateLimitedRequester(basicRequester, riotRateLimitProvider); | ||
|
||
serviceCollection.AddSingleton<ITournamentRiotApi>(serviceProvider => new TournamentRiotApi(tournamentRateLimitedRequester, riotSharpOptions.TournamentApi.UseStub)); | ||
} | ||
|
||
return serviceCollection; | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Net.Http; | ||
using System.Text; | ||
using RiotSharp.Http; | ||
using RiotSharp.Http.Interfaces; | ||
|
||
namespace RiotSharp.Test | ||
{ | ||
class RateLimitedRequesterTestDoubles | ||
{ | ||
public static IRateLimitedRequester Real(string apiKey, IDictionary<TimeSpan, int> rateLimits) | ||
{ | ||
var serializer = new RequestContentSerializer(); | ||
var deserializer = new ResponseDeserializer(); | ||
var requestCreator = new RequestCreator(apiKey, serializer); | ||
var httpClient = new HttpClient(); | ||
var failedRequestHandler = new FailedRequestHandler(); | ||
var client = new RequestClient(httpClient, failedRequestHandler); | ||
var basicRequester = new Requester(client, requestCreator, deserializer); | ||
var rateLimitProvider = new RateLimitProvider(rateLimits); | ||
|
||
return new RateLimitedRequester(basicRequester, rateLimitProvider); | ||
} | ||
|
||
public static IRateLimitedRequester Real(string apiKey) | ||
{ | ||
var rateLimits = new Dictionary<TimeSpan, int> { { new TimeSpan(1, 0, 0), 10 } }; | ||
|
||
return Real(apiKey, rateLimits); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Net.Http; | ||
using System.Text; | ||
using RiotSharp.Http; | ||
using RiotSharp.Http.Interfaces; | ||
|
||
namespace RiotSharp.Test | ||
{ | ||
public class RequesterTestDoubles | ||
{ | ||
public static IRequester Real(string apiKey) | ||
{ | ||
var serializer = new RequestContentSerializer(); | ||
var deserializer = new ResponseDeserializer(); | ||
var requestCreator = new RequestCreator(apiKey, serializer); | ||
|
||
var httpClient = new HttpClient(); | ||
var failedRequestHandler = new FailedRequestHandler(); | ||
|
||
var client = new RequestClient(httpClient, failedRequestHandler); | ||
|
||
return new Requester(client, requestCreator, deserializer); | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Net; | ||
using System.Net.Http; | ||
using System.Text; | ||
using RiotSharp.Http.Interfaces; | ||
|
||
namespace RiotSharp.Http | ||
{ | ||
public class FailedRequestHandler : IFailedRequestHandler | ||
{ | ||
public void Handle(HttpResponseMessage message) | ||
{ | ||
if (message.IsSuccessStatusCode) | ||
{ | ||
return; | ||
} | ||
switch (message.StatusCode) | ||
{ | ||
case HttpStatusCode.ServiceUnavailable: | ||
throw new RiotSharpException("503, Service unavailable", message.StatusCode); | ||
case HttpStatusCode.InternalServerError: | ||
throw new RiotSharpException("500, Internal server error", message.StatusCode); | ||
case HttpStatusCode.Unauthorized: | ||
throw new RiotSharpException("401, Unauthorized", message.StatusCode); | ||
case HttpStatusCode.BadRequest: | ||
throw new RiotSharpException("400, Bad request", message.StatusCode); | ||
case HttpStatusCode.NotFound: | ||
throw new RiotSharpException("404, Resource not found", message.StatusCode); | ||
case HttpStatusCode.Forbidden: | ||
throw new RiotSharpException("403, Forbidden", message.StatusCode); | ||
case (HttpStatusCode)429: | ||
throw new RiotSharpException("429, Rate Limit Exceeded", message.StatusCode); | ||
default: | ||
throw new RiotSharpException("Unexpeced failure", message.StatusCode); | ||
} | ||
} | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
using System.Net.Http; | ||
|
||
namespace RiotSharp.Http.Interfaces | ||
{ | ||
public interface IFailedRequestHandler | ||
{ | ||
void Handle(HttpResponseMessage message); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Text; | ||
using RiotSharp.Misc; | ||
|
||
namespace RiotSharp.Http.Interfaces | ||
{ | ||
public interface IRateLimitProvider | ||
{ | ||
RateLimiter GetLimiter(Region region); | ||
} | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could we remove the double empty lines in this file