From a02518c8aa768fdfb783ca97bb08b5c9c563093c Mon Sep 17 00:00:00 2001 From: Jason Regnier Date: Wed, 10 Oct 2018 14:31:34 -0400 Subject: [PATCH] Fixed issue with the Rate limit manager where tests would wait forever --- .../Service/CardServiceTest.cs | 20 ++--- .../Service/ServiceBaseObjectService.cs | 4 +- .../Service/SetServiceTest.cs | 12 +-- MtgApiManager.Lib/Core/MtgApiController.cs | 86 +++++++++++++------ MtgApiManager.Lib/Core/RateLimit.cs | 2 +- MtgApiManager.Lib/Service/CardService.cs | 18 ++-- MtgApiManager.Lib/Service/ServiceBase.cs | 60 ++++--------- MtgApiManager.Lib/Service/SetService.cs | 13 +-- 8 files changed, 109 insertions(+), 106 deletions(-) diff --git a/MtgApiManager.Lib.Test/Service/CardServiceTest.cs b/MtgApiManager.Lib.Test/Service/CardServiceTest.cs index 7f3f762..d203853 100644 --- a/MtgApiManager.Lib.Test/Service/CardServiceTest.cs +++ b/MtgApiManager.Lib.Test/Service/CardServiceTest.cs @@ -131,7 +131,7 @@ public async Task AllAsyncTest() .Throws() .ReturnsAsync(new RootCardListDto() { Cards = cards }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); service = service.Where(x => x.Name, "name1"); var result = await service.AllAsync(); @@ -275,7 +275,7 @@ public void AllTest() .Throws() .ReturnsAsync(new RootCardListDto() { Cards = cards }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); service = service.Where(x => x.Name, "name1"); var result = service.All(); @@ -375,7 +375,7 @@ public async Task FindAsyncTest() .Throws() .ReturnsAsync(new RootCardDto() { Card = cardDto }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = await service.FindAsync(1); Assert.False(result.IsSuccess); @@ -518,7 +518,7 @@ public void FindTest() .Throws() .ReturnsAsync(new RootCardDto() { Card = cardDto }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = service.Find(1); Assert.False(result.IsSuccess); @@ -628,7 +628,7 @@ public async Task GetCardSubTypesAsyncTest() .Throws() .ReturnsAsync(new RootCardSubTypeDto() { SubTypes = cardSubTypes }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = await service.GetCardSubTypesAsync(); Assert.False(result.IsSuccess); @@ -692,7 +692,7 @@ public void GetCardSubTypesTest() .Throws() .ReturnsAsync(new RootCardSubTypeDto() { SubTypes = cardSubTypes }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = service.GetCardSubTypes(); Assert.False(result.IsSuccess); @@ -757,7 +757,7 @@ public async Task GetCardSuperTypesAsyncTest() .Throws() .ReturnsAsync(new RootCardSuperTypeDto() { SuperTypes = cardSuperTypes }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = await service.GetCardSuperTypesAsync(); Assert.False(result.IsSuccess); @@ -821,7 +821,7 @@ public void GetCardSuperTypesTest() .Throws() .ReturnsAsync(new RootCardSuperTypeDto() { SuperTypes = cardSuperTypes }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = service.GetCardSuperTypes(); Assert.False(result.IsSuccess); @@ -886,7 +886,7 @@ public async Task GetCardTypesAsyncTest() .Throws() .ReturnsAsync(new RootCardTypeDto() { Types = cardTypes }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = await service.GetCardTypesAsync(); Assert.False(result.IsSuccess); @@ -950,7 +950,7 @@ public void GetCardTypesTest() .Throws() .ReturnsAsync(new RootCardTypeDto() { Types = cardTypes }); - var service = new CardService(moqAdapter.Object); + var service = new CardService(moqAdapter.Object, ApiVersion.V1_0, false); var result = service.GetCardTypes(); Assert.False(result.IsSuccess); diff --git a/MtgApiManager.Lib.Test/Service/ServiceBaseObjectService.cs b/MtgApiManager.Lib.Test/Service/ServiceBaseObjectService.cs index ece88de..bb84ef3 100644 --- a/MtgApiManager.Lib.Test/Service/ServiceBaseObjectService.cs +++ b/MtgApiManager.Lib.Test/Service/ServiceBaseObjectService.cs @@ -21,7 +21,7 @@ public class ServiceBaseObjectService : ServiceBase /// Initializes a new instance of the class. Defaults to version 1.0 of the API. /// public ServiceBaseObjectService() - : base(new MtgApiServiceAdapter(), ApiVersion.V1_0, ApiEndPoint.Cards) + : base(new MtgApiServiceAdapter(), ApiVersion.V1_0, ApiEndPoint.Cards, false) { } @@ -30,7 +30,7 @@ public ServiceBaseObjectService() /// /// The adapter to use. public ServiceBaseObjectService(IMtgApiServiceAdapter adapter) - : base(adapter, ApiVersion.V1_0, ApiEndPoint.Cards) + : base(adapter, ApiVersion.V1_0, ApiEndPoint.Cards, false) { } diff --git a/MtgApiManager.Lib.Test/Service/SetServiceTest.cs b/MtgApiManager.Lib.Test/Service/SetServiceTest.cs index ec8310b..ce84da0 100644 --- a/MtgApiManager.Lib.Test/Service/SetServiceTest.cs +++ b/MtgApiManager.Lib.Test/Service/SetServiceTest.cs @@ -102,7 +102,7 @@ public async Task AllAsyncTest() .Throws() .ReturnsAsync(new RootSetListDto() { Sets = sets }); - var service = new SetService(moqAdapter.Object); + var service = new SetService(moqAdapter.Object, ApiVersion.V1_0, false); service = service.Where(x => x.Name, "name1"); var result = await service.AllAsync(); @@ -211,7 +211,7 @@ public void AllTest() .Throws() .ReturnsAsync(new RootSetListDto() { Sets = sets }); - var service = new SetService(moqAdapter.Object); + var service = new SetService(moqAdapter.Object, ApiVersion.V1_0, false); service = service.Where(x => x.Name, "name1"); var result = service.All(); @@ -296,7 +296,7 @@ public async Task FindAsyncTest() .Throws() .ReturnsAsync(new RootSetDto() { Set = setDto }); - var service = new SetService(moqAdapter.Object); + var service = new SetService(moqAdapter.Object, ApiVersion.V1_0, false); var result = await service.FindAsync("code1"); Assert.False(result.IsSuccess); @@ -379,7 +379,7 @@ public void FindTest() .Throws() .ReturnsAsync(new RootSetDto() { Set = setDto }); - var service = new SetService(moqAdapter.Object); + var service = new SetService(moqAdapter.Object, ApiVersion.V1_0, false); var result = service.Find("code1"); Assert.False(result.IsSuccess); @@ -523,7 +523,7 @@ public async Task GenerateBoosterAsyncTest() .Throws() .ReturnsAsync(new RootCardListDto() { Cards = cards }); - var service = new SetService(moqAdapter.Object); + var service = new SetService(moqAdapter.Object, ApiVersion.V1_0, false); var result = await service.GenerateBoosterAsync("ktk"); Assert.False(result.IsSuccess); @@ -666,7 +666,7 @@ public void GenerateBoosterTest() .Throws() .ReturnsAsync(new RootCardListDto() { Cards = cards }); - var service = new SetService(moqAdapter.Object); + var service = new SetService(moqAdapter.Object, ApiVersion.V1_0, false); var result = service.GenerateBooster("ktk"); Assert.False(result.IsSuccess); diff --git a/MtgApiManager.Lib/Core/MtgApiController.cs b/MtgApiManager.Lib/Core/MtgApiController.cs index 1d86361..a4f9dca 100644 --- a/MtgApiManager.Lib/Core/MtgApiController.cs +++ b/MtgApiManager.Lib/Core/MtgApiController.cs @@ -4,6 +4,7 @@ using System.Linq; using System.Net.Http.Headers; using System.Runtime.CompilerServices; +using System.Threading; using System.Threading.Tasks; [assembly: InternalsVisibleTo("MtgApiManager.Lib.Test")] @@ -13,6 +14,8 @@ /// internal static class MtgApiController { + private static readonly SemaphoreSlim _semaphoreSlim = new SemaphoreSlim(1, 1); + /// /// The rate limit which controls the calls to the API. /// @@ -79,14 +82,32 @@ public static PagingInfo CreatePagingInfo() public async static Task HandleRateLimit() { - int delayInMilliseconds = _apiRateLimit.GetDelay(MtgApiController.RatelimitLimit); + await _semaphoreSlim.WaitAsync().ConfigureAwait(false); + + try + { + int delayInMilliseconds = _apiRateLimit.GetDelay(RatelimitLimit); - if (delayInMilliseconds > 0) + if (delayInMilliseconds > 0) + { + await Task.Delay(delayInMilliseconds).ConfigureAwait(false); + } + + _apiRateLimit.AddApiCall(); + } + finally { - await Task.Delay(delayInMilliseconds); + _semaphoreSlim.Release(); } + } - _apiRateLimit.AddApiCall(); + public static void ResetRateLimit() + { + _semaphoreSlim.Wait(); + + _apiRateLimit = new RateLimit(); + + _semaphoreSlim.Release(); } /// @@ -100,34 +121,43 @@ public static void ParseHeaders(HttpResponseHeaders headers) throw new ArgumentNullException("headers"); } - if (headers.TryGetValues("Link", out IEnumerable resultHeaders)) - { - Link = resultHeaders.FirstOrDefault(); - } + _semaphoreSlim.Wait(); - if (headers.TryGetValues("Page-Size", out resultHeaders)) + try { - PageSize = int.Parse(resultHeaders.FirstOrDefault()); + if (headers.TryGetValues("Link", out IEnumerable resultHeaders)) + { + Link = resultHeaders.FirstOrDefault(); + } + + if (headers.TryGetValues("Page-Size", out resultHeaders)) + { + PageSize = int.Parse(resultHeaders.FirstOrDefault()); + } + + if (headers.TryGetValues("Count", out resultHeaders)) + { + Count = int.Parse(resultHeaders.FirstOrDefault()); + } + + if (headers.TryGetValues("Total-Count", out resultHeaders)) + { + TotalCount = int.Parse(resultHeaders.FirstOrDefault()); + } + + if (headers.TryGetValues("Ratelimit-Limit", out resultHeaders)) + { + RatelimitLimit = int.Parse(resultHeaders.FirstOrDefault()); + } + + if (headers.TryGetValues("Ratelimit-Remaining", out resultHeaders)) + { + RatelimitRemaining = int.Parse(resultHeaders.FirstOrDefault()); + } } - - if (headers.TryGetValues("Count", out resultHeaders)) - { - Count = int.Parse(resultHeaders.FirstOrDefault()); - } - - if (headers.TryGetValues("Total-Count", out resultHeaders)) - { - TotalCount = int.Parse(resultHeaders.FirstOrDefault()); - } - - if (headers.TryGetValues("Ratelimit-Limit", out resultHeaders)) - { - RatelimitLimit = int.Parse(resultHeaders.FirstOrDefault()); - } - - if (headers.TryGetValues("Ratelimit-Remaining", out resultHeaders)) + finally { - RatelimitRemaining = int.Parse(resultHeaders.FirstOrDefault()); + _semaphoreSlim.Release(); } } } \ No newline at end of file diff --git a/MtgApiManager.Lib/Core/RateLimit.cs b/MtgApiManager.Lib/Core/RateLimit.cs index 160d023..c63b466 100644 --- a/MtgApiManager.Lib/Core/RateLimit.cs +++ b/MtgApiManager.Lib/Core/RateLimit.cs @@ -16,7 +16,7 @@ internal class RateLimit /// /// The calls that have been made to the web service. /// - private List _webServiceCalls; + private readonly List _webServiceCalls; /// /// Initializes a new instance of the class. diff --git a/MtgApiManager.Lib/Service/CardService.cs b/MtgApiManager.Lib/Service/CardService.cs index a136404..666a30d 100644 --- a/MtgApiManager.Lib/Service/CardService.cs +++ b/MtgApiManager.Lib/Service/CardService.cs @@ -24,7 +24,7 @@ public class CardService /// /// The list of queries to apply. /// - private NameValueCollection _whereQueries = null; + private readonly NameValueCollection _whereQueries; /// /// Initializes a new instance of the class. Defaults to version 1.0 of the API. @@ -48,8 +48,9 @@ public CardService() /// /// The service adapter used to interact with the MTG API. /// The version of the API - public CardService(IMtgApiServiceAdapter serviceAdapter, ApiVersion version) - : base(serviceAdapter, version, ApiEndPoint.Cards) + /// Turn the rate limit on or off. + public CardService(IMtgApiServiceAdapter serviceAdapter, ApiVersion version, bool rateLimitOn = true) + : base(serviceAdapter, version, ApiEndPoint.Cards, rateLimitOn) { _whereQueries = new NameValueCollection(); } @@ -63,7 +64,7 @@ public static List MapCardsList(RootCardListDto cardListDto) { if (cardListDto == null) { - throw new ArgumentNullException("cardListDto"); + throw new ArgumentNullException(nameof(cardListDto)); } if (cardListDto.Cards == null) @@ -319,12 +320,12 @@ public CardService Where(Expression> property, U { if (property == null) { - throw new ArgumentNullException("property"); + throw new ArgumentNullException(nameof(property)); } - if (value == null) + if (EqualityComparer.Default.Equals(value, default(U))) { - throw new ArgumentNullException("value"); + throw new ArgumentNullException(nameof(value)); } MemberExpression expression = property.Body as MemberExpression; @@ -333,8 +334,7 @@ public CardService Where(Expression> property, U Type valueType = value.GetType(); if (valueType.IsArray) { - string val = string.Join("|", (IEnumerable)value); - _whereQueries[queryName] = val; + _whereQueries[queryName] = string.Join("|", (IEnumerable)value); } else { diff --git a/MtgApiManager.Lib/Service/ServiceBase.cs b/MtgApiManager.Lib/Service/ServiceBase.cs index 3a9d114..db2d78b 100644 --- a/MtgApiManager.Lib/Service/ServiceBase.cs +++ b/MtgApiManager.Lib/Service/ServiceBase.cs @@ -26,20 +26,7 @@ public abstract class ServiceBase /// protected const string BaseMtgUrl = "https://api.magicthegathering.io"; - /// - /// The adapter used to interact with the MTG API. - /// - private IMtgApiServiceAdapter _adapter = null; - - /// - /// The end point for the service. - /// - private ApiEndPoint _endpoint; - - /// - /// The version of the API. - /// - private ApiVersion _version; + private readonly bool _isRateLimitOn; /// /// Initializes a new instance of the class. @@ -47,45 +34,29 @@ public abstract class ServiceBase /// The service adapter used to interact with the MTG API. /// The version of the API (currently only 1 version.) /// The end point of the service. - public ServiceBase(IMtgApiServiceAdapter serviceAdapter, ApiVersion version, ApiEndPoint endpoint) + /// Turn the rate limit on or off. + public ServiceBase(IMtgApiServiceAdapter serviceAdapter, ApiVersion version, ApiEndPoint endpoint, bool rateLimitOn) { - _adapter = serviceAdapter; - _version = version; - _endpoint = endpoint; + Adapter = serviceAdapter; + Version = version; + EndPoint = endpoint; + _isRateLimitOn = rateLimitOn; } /// /// Gets the adapter used to interact with the MTG API. /// - protected IMtgApiServiceAdapter Adapter - { - get - { - return _adapter; - } - } + protected IMtgApiServiceAdapter Adapter { get; } /// /// Gets the end point of the service. /// - protected ApiEndPoint EndPoint - { - get - { - return _endpoint; - } - } + protected ApiEndPoint EndPoint { get; } /// /// Gets the version of the MTG API. /// - protected ApiVersion Version - { - get - { - return _version; - } - } + protected ApiVersion Version { get; } /// /// Gets all the defined by the query parameters. @@ -114,7 +85,7 @@ protected Uri BuildUri(NameValueCollection parameters) var urlBuilder = new UriBuilder( new Uri( new Uri(BaseMtgUrl), - string.Concat(_version.GetDescription(), "/", _endpoint.GetDescription()))); + string.Concat(Version.GetDescription(), "/", EndPoint.GetDescription()))); var query = HttpUtility.ParseQueryString(urlBuilder.Query); query.Add(parameters); @@ -137,7 +108,7 @@ protected Uri BuildUri(string parameterValue) return new Uri( new Uri(BaseMtgUrl), - string.Concat(_version.GetDescription(), "/", _endpoint.GetDescription(), "/", parameterValue)); + string.Concat(Version.GetDescription(), "/", EndPoint.GetDescription(), "/", parameterValue)); } /// @@ -154,10 +125,11 @@ protected async Task CallWebServiceGet(Uri requestUri) throw new ArgumentNullException("requestUri"); } - // Makes sure that th rate limit is not reached. - await MtgApiController.HandleRateLimit(); + // Makes sure that th rate limit is not reached. + if (_isRateLimitOn) + await MtgApiController.HandleRateLimit().ConfigureAwait(false); - return await _adapter.WebGetAsync(requestUri).ConfigureAwait(false); + return await Adapter.WebGetAsync(requestUri).ConfigureAwait(false); } } } \ No newline at end of file diff --git a/MtgApiManager.Lib/Service/SetService.cs b/MtgApiManager.Lib/Service/SetService.cs index 891d865..5a2745a 100644 --- a/MtgApiManager.Lib/Service/SetService.cs +++ b/MtgApiManager.Lib/Service/SetService.cs @@ -50,8 +50,9 @@ public SetService() /// /// The service adapter used to interact with the MTG API. /// The version of the API - public SetService(IMtgApiServiceAdapter serviceAdapter, ApiVersion version) - : base(serviceAdapter, version, ApiEndPoint.Sets) + /// Turn the rate limit on or off. + public SetService(IMtgApiServiceAdapter serviceAdapter, ApiVersion version, bool rateLimitOn = true) + : base(serviceAdapter, version, ApiEndPoint.Sets, rateLimitOn) { _whereQueries = new NameValueCollection(); } @@ -65,7 +66,7 @@ public static List MapSetsList(RootSetListDto setListDto) { if (setListDto == null) { - throw new ArgumentNullException("setListDto"); + throw new ArgumentNullException(nameof(setListDto)); } if (setListDto.Sets == null) @@ -207,12 +208,12 @@ public SetService Where(Expression> property, U va { if (property == null) { - throw new ArgumentNullException("property"); + throw new ArgumentNullException(nameof(property)); } - if (value == null) + if (EqualityComparer.Default.Equals(value, default(U))) { - throw new ArgumentNullException("value"); + throw new ArgumentNullException(nameof(value)); } MemberExpression expression = property.Body as MemberExpression;