From 86097d0b452f9acdd6d91537c1e4950ca1b3ec38 Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Wed, 18 Nov 2020 17:11:03 +0100 Subject: [PATCH 1/9] Some refactoring to make the code a bit better divided in parts with their own responsibility. --- .../Sdk/Api/RequestSender/ApiRequestSender.cs | 89 +++++++++---------- 1 file changed, 42 insertions(+), 47 deletions(-) diff --git a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs index e32903e..052d508 100644 --- a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs +++ b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs @@ -7,12 +7,12 @@ using System.Net.Http.Headers; using System.Threading; using System.Threading.Tasks; -using Bynder.Sdk.Model; using Bynder.Sdk.Api.Requests; -using Newtonsoft.Json; -using Bynder.Sdk.Query.Decoder; +using Bynder.Sdk.Model; using Bynder.Sdk.Query; +using Bynder.Sdk.Query.Decoder; using Bynder.Sdk.Settings; +using Newtonsoft.Json; namespace Bynder.Sdk.Api.RequestSender { @@ -38,8 +38,8 @@ internal class ApiRequestSender : IApiRequestSender internal ApiRequestSender(Configuration configuration, ICredentials credentials, IHttpRequestSender httpSender) { _configuration = configuration; - _httpSender = httpSender; _credentials = credentials; + _httpSender = httpSender; } /// @@ -72,26 +72,9 @@ public void Dispose() /// Check . /// Check . /// Check . - public async Task SendRequestAsync(Requests.Request request) + public async Task SendRequestAsync(Request request) { - if (request.Authenticated && !_credentials.AreValid()) - { - await _semaphore.WaitAsync().ConfigureAwait(false); - if (!_credentials.AreValid()) - { - try - { - await RefreshToken().ConfigureAwait(false); - } - finally - { - _semaphore.Release(); - } - } - } - - var httpRequest = CreateHttpRequest(request); - var response = await _httpSender.SendHttpRequest(httpRequest).ConfigureAwait(false); + var response = await CreateHttpRequestAsync(request).ConfigureAwait(false); var responseContent = response.Content; if (response.Content == null) @@ -100,7 +83,7 @@ public async Task SendRequestAsync(Requests.Request request) } var responseString = await responseContent.ReadAsStringAsync().ConfigureAwait(false); - if (string.IsNullOrEmpty(responseString)) + if (responseString == null) { return default(T); } @@ -108,45 +91,57 @@ public async Task SendRequestAsync(Requests.Request request) return JsonConvert.DeserializeObject(responseString); } - private HttpRequestMessage CreateHttpRequest(Requests.Request request) + private async Task CreateHttpRequestAsync(Request request) { var parameters = _queryDecoder.GetParameters(request.Query); var httpRequestMessage = HttpRequestMessageFactory.Create( _configuration.BaseUrl.ToString(), request.HTTPMethod, parameters, - request.Path); + request.Path + ); if (request.Authenticated) { - httpRequestMessage.Headers.Authorization = - new AuthenticationHeaderValue(_credentials.TokenType, _credentials.AccessToken); + if (!_credentials.AreValid()) + { + _credentials.Update(await RefreshToken().ConfigureAwait(false)); + } + + httpRequestMessage.Headers.Authorization = new AuthenticationHeaderValue( + _credentials.TokenType, + _credentials.AccessToken + ); } - return httpRequestMessage; + return await _httpSender.SendHttpRequest(httpRequestMessage).ConfigureAwait(false); } - private async Task RefreshToken() + private async Task RefreshToken() { - TokenQuery query = new TokenQuery + await _semaphore.WaitAsync().ConfigureAwait(false); + try { - ClientId = _configuration.ClientId, - ClientSecret = _configuration.ClientSecret, - RefreshToken = _credentials.RefreshToken, - GrantType = "refresh_token" - }; - - var request = new OAuthRequest + return await SendRequestAsync( + new OAuthRequest + { + Authenticated = false, + Query = new TokenQuery + { + ClientId = _configuration.ClientId, + ClientSecret = _configuration.ClientSecret, + RefreshToken = _credentials.RefreshToken, + GrantType = "refresh_token" + }, + Path = TokenPath, + HTTPMethod = HttpMethod.Post + } + ).ConfigureAwait(false); + } + finally { - Authenticated = false, - Query = query, - Path = TokenPath, - HTTPMethod = HttpMethod.Post - }; - - var newToken = await SendRequestAsync(request).ConfigureAwait(false); - - _credentials.Update(newToken); + _semaphore.Release(); + } } private static class HttpRequestMessageFactory From b35270c01e5ab5e7313d1e939a4e9e55d478bace Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 10:41:19 +0100 Subject: [PATCH 2/9] Also test response body and exception handling --- .../Api/RequestSender/ApiRequestSenderTest.cs | 180 +++++++++++++++--- 1 file changed, 153 insertions(+), 27 deletions(-) diff --git a/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs b/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs index 8d98bdc..daf97a4 100644 --- a/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs +++ b/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs @@ -2,13 +2,17 @@ // Licensed under the MIT License. See LICENSE file in the project root for full license information. using System; +using System.Collections.Generic; +using System.Linq; using System.Net.Http; +using System.Text; using System.Threading.Tasks; using Bynder.Sdk.Api.Requests; using Bynder.Sdk.Api.RequestSender; using Bynder.Sdk.Query.Decoder; using Bynder.Sdk.Settings; using Moq; +using Newtonsoft.Json; using Xunit; namespace Bynder.Test.Api.RequestSender @@ -23,23 +27,36 @@ public class ApiRequestSenderTest private Mock _httpSenderMock; private StubQuery _query; + private IList _expectedResponseBody; public ApiRequestSenderTest() { _httpSenderMock = new Mock(); - _httpSenderMock - .Setup(sender => sender.SendHttpRequest(It.IsAny())) - .Returns(Task.FromResult(new HttpResponseMessage(System.Net.HttpStatusCode.OK))); _query = new StubQuery { Item1 = _queryValue }; + _expectedResponseBody = new List { "foo", "bar" }; } [Fact] - public async Task WhenRequestIsPostThenParametersAreAddedToContent() + public async Task WhenErrorResponseThenReturnDefaultObject() { - await SendRequest(hasValidCredentials: true, httpMethod: HttpMethod.Post); + var expectedResponse = new HttpResponseMessage + { + StatusCode = System.Net.HttpStatusCode.InternalServerError, + Content = new StringContent( + JsonConvert.SerializeObject(_expectedResponseBody), + Encoding.UTF8, + "application/json" + ) + }; + var doRequest = SendRequestAsync>( + hasValidCredentials: true, + httpMethod: HttpMethod.Post, + expectedResponse + ); + await Assert.ThrowsAsync(() => doRequest); _httpSenderMock.Verify(sender => sender.SendHttpRequest( It.Is(req => @@ -48,25 +65,41 @@ public async Task WhenRequestIsPostThenParametersAreAddedToContent() && req.Headers.Authorization.ToString() == _authHeader && req.Content.ReadAsStringAsync().Result.Contains(_queryString) ) - )); - - _httpSenderMock.Verify(sender => sender.SendHttpRequest( - It.IsAny() ), Times.Once); } - [Fact] - public async Task WhenCredentialInvalidTwoRequestsSent() + public async Task WhenEmptyResponseThenReturnDefaultObject() { - await SendRequest(hasValidCredentials: false, httpMethod: HttpMethod.Get); + var responseBody = await SendRequestAsync>( + hasValidCredentials: true, + httpMethod: HttpMethod.Post, + System.Net.HttpStatusCode.OK + ); + + Assert.Equal(default(IList), responseBody); _httpSenderMock.Verify(sender => sender.SendHttpRequest( - It.Is( - req => req.RequestUri.PathAndQuery == ApiRequestSender.TokenPath + It.Is(req => + req.RequestUri.PathAndQuery.Contains(_path) && req.Method == HttpMethod.Post + && req.Headers.Authorization.ToString() == _authHeader + && req.Content.ReadAsStringAsync().Result.Contains(_queryString) ) - )); + ), Times.Once); + } + + [Fact] + public async Task WhenRequestIsGetThenParametersAreAddedToUrl() + { + var responseBody = await SendRequestAsync( + hasValidCredentials: true, + httpMethod: HttpMethod.Get, + System.Net.HttpStatusCode.OK, + _expectedResponseBody + ); + + Assert.Equal(_expectedResponseBody, responseBody); _httpSenderMock.Verify(sender => sender.SendHttpRequest( It.Is( @@ -75,18 +108,52 @@ public async Task WhenCredentialInvalidTwoRequestsSent() && req.Headers.Authorization.ToString() == _authHeader && req.RequestUri.Query.Contains(_queryString) ) - )); + ), Times.Once); + } + + [Fact] + public async Task WhenRequestIsPostThenParametersAreAddedToContent() + { + var responseBody = await SendRequestAsync( + hasValidCredentials: true, + httpMethod: HttpMethod.Post, + System.Net.HttpStatusCode.OK, + _expectedResponseBody + ); + + Assert.Equal(_expectedResponseBody, responseBody); _httpSenderMock.Verify(sender => sender.SendHttpRequest( - It.IsAny() - ), Times.Exactly(2)); + It.Is(req => + req.RequestUri.PathAndQuery.Contains(_path) + && req.Method == HttpMethod.Post + && req.Headers.Authorization.ToString() == _authHeader + && req.Content.ReadAsStringAsync().Result.Contains(_queryString) + ) + ), Times.Once); } [Fact] - public async Task WhenRequestIsGetThenParametersAreAddedToUrl() + public async Task WhenCredentialInvalidThenTokenIsRefreshed() { - await SendRequest(hasValidCredentials: true, httpMethod: HttpMethod.Get); + var responseBody = await SendRequestAsync( + hasValidCredentials: false, + httpMethod: HttpMethod.Get, + System.Net.HttpStatusCode.OK, + _expectedResponseBody + ); + Assert.Equal(_expectedResponseBody, responseBody); + + // Check Refresh token request + _httpSenderMock.Verify(sender => sender.SendHttpRequest( + It.Is( + req => req.RequestUri.PathAndQuery == ApiRequestSender.TokenPath + && req.Method == HttpMethod.Post + ) + ), Times.Once); + + // Check API request _httpSenderMock.Verify(sender => sender.SendHttpRequest( It.Is( req => req.RequestUri.PathAndQuery.Contains(_path) @@ -94,17 +161,76 @@ public async Task WhenRequestIsGetThenParametersAreAddedToUrl() && req.Headers.Authorization.ToString() == _authHeader && req.RequestUri.Query.Contains(_queryString) ) - )); - - _httpSenderMock.Verify(sender => sender.SendHttpRequest( - It.IsAny() ), Times.Once); } - private async Task SendRequest(bool hasValidCredentials, HttpMethod httpMethod) + private async Task SendRequestAsync( + bool hasValidCredentials, + HttpMethod httpMethod, + System.Net.HttpStatusCode responseStatus) { - await CreateApiRequestSender(hasValidCredentials).SendRequestAsync( - new ApiRequest() + return await SendRequestAsync( + hasValidCredentials, + httpMethod, + new HttpResponseMessage + { + StatusCode = responseStatus + } + ); + } + + private async Task SendRequestAsync( + bool hasValidCredentials, + HttpMethod httpMethod, + System.Net.HttpStatusCode responseStatus, + T responseBody) + { + return await SendRequestAsync( + hasValidCredentials, + httpMethod, + new HttpResponseMessage + { + StatusCode = responseStatus, + Content = new StringContent( + JsonConvert.SerializeObject(responseBody), + Encoding.UTF8, + "application/json" + ) + } + ); + } + + private async Task SendRequestAsync( + bool hasValidCredentials, + HttpMethod httpMethod, + HttpResponseMessage response) + { + var httpSenderMockSetup = _httpSenderMock + .Setup(sender => sender.SendHttpRequest( + It.Is(req => req.RequestUri.PathAndQuery != ApiRequestSender.TokenPath) + )); + if (response.IsSuccessStatusCode) + { + httpSenderMockSetup.Returns(Task.FromResult(response)); + } + else + { + httpSenderMockSetup.ThrowsAsync(new HttpRequestException("")); + } + + _httpSenderMock + .Setup(sender => sender.SendHttpRequest( + It.Is(req => req.RequestUri.PathAndQuery == ApiRequestSender.TokenPath) + )) + .Returns(Task.FromResult( + new HttpResponseMessage + { + StatusCode = System.Net.HttpStatusCode.OK + } + )); + + return await CreateApiRequestSender(hasValidCredentials).SendRequestAsync( + new ApiRequest { Path = _path, HTTPMethod = httpMethod, From 9ab8f3f69a91a71c9c07dd4b5317fd56e75955ab Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 10:42:26 +0100 Subject: [PATCH 3/9] Added exception info to docstrings. --- Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs | 3 ++- Bynder/Sdk/Api/RequestSender/HttpRequestSender.cs | 3 ++- Bynder/Sdk/Api/RequestSender/IApiRequestSender.cs | 4 +++- Bynder/Sdk/Api/RequestSender/IHttpRequestSender.cs | 4 +++- Bynder/Sdk/Service/Asset/IAssetService.cs | 1 + 5 files changed, 11 insertions(+), 4 deletions(-) diff --git a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs index 052d508..2e0327b 100644 --- a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs +++ b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs @@ -69,9 +69,10 @@ public void Dispose() /// /// Check . /// - /// Check . /// Check . /// Check . + /// Check . + /// Check . public async Task SendRequestAsync(Request request) { var response = await CreateHttpRequestAsync(request).ConfigureAwait(false); diff --git a/Bynder/Sdk/Api/RequestSender/HttpRequestSender.cs b/Bynder/Sdk/Api/RequestSender/HttpRequestSender.cs index 3de9ecf..7a99924 100644 --- a/Bynder/Sdk/Api/RequestSender/HttpRequestSender.cs +++ b/Bynder/Sdk/Api/RequestSender/HttpRequestSender.cs @@ -26,8 +26,9 @@ public string UserAgent /// /// Sends the HTTP request and returns its response. /// - /// The HTTP request response. /// HTTP request. + /// The HTTP request response. + /// Check . public async Task SendHttpRequest(HttpRequestMessage httpRequest) { httpRequest.Headers.Add("User-Agent", UserAgent); diff --git a/Bynder/Sdk/Api/RequestSender/IApiRequestSender.cs b/Bynder/Sdk/Api/RequestSender/IApiRequestSender.cs index 4b289cf..c37adb2 100644 --- a/Bynder/Sdk/Api/RequestSender/IApiRequestSender.cs +++ b/Bynder/Sdk/Api/RequestSender/IApiRequestSender.cs @@ -17,9 +17,11 @@ internal interface IApiRequestSender : IDisposable /// /// Sends the request async. /// - /// The deserialized response. /// Request. /// Type we want to deserialize response to. + /// The deserialized response. + /// The request failed due to an underlying issue + /// such as network connectivity, DNS failure, server certificate validation or timeout. Task SendRequestAsync(Request request); } } diff --git a/Bynder/Sdk/Api/RequestSender/IHttpRequestSender.cs b/Bynder/Sdk/Api/RequestSender/IHttpRequestSender.cs index d46f2b4..8390283 100644 --- a/Bynder/Sdk/Api/RequestSender/IHttpRequestSender.cs +++ b/Bynder/Sdk/Api/RequestSender/IHttpRequestSender.cs @@ -16,8 +16,10 @@ internal interface IHttpRequestSender : IDisposable /// /// Sends the HTTP request and returns its response. /// - /// The HTTP request response. /// HTTP request. + /// The HTTP request response. + /// The request failed due to an underlying issue + /// such as network connectivity, DNS failure, server certificate validation or timeout. Task SendHttpRequest(HttpRequestMessage httpRequest); } } diff --git a/Bynder/Sdk/Service/Asset/IAssetService.cs b/Bynder/Sdk/Service/Asset/IAssetService.cs index 6fc02e6..c8aa339 100644 --- a/Bynder/Sdk/Service/Asset/IAssetService.cs +++ b/Bynder/Sdk/Service/Asset/IAssetService.cs @@ -19,6 +19,7 @@ public interface IAssetService /// Gets Brands Async /// /// Task with list of brands + /// Can be thrown when requests to server can't be completed or HTTP code returned by server is an error Task> GetBrandsAsync(); /// From 10c72a7dda511ee66e57bf31ffdf89408b606134 Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 10:44:50 +0100 Subject: [PATCH 4/9] Version bump & release notes --- Bynder/Sdk/Bynder.Sdk.csproj | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Bynder/Sdk/Bynder.Sdk.csproj b/Bynder/Sdk/Bynder.Sdk.csproj index ac17e6e..a34be52 100644 --- a/Bynder/Sdk/Bynder.Sdk.csproj +++ b/Bynder/Sdk/Bynder.Sdk.csproj @@ -7,7 +7,7 @@ Bynder.Sdk Copyright © Bynder true - 2.2.0 + 2.2.1 BynderDevops The main goal of this SDK is to speed up the integration of Bynder customers who use C# making it easier to connect to the Bynder API (http://docs.bynder.apiary.io/) and executing requests on it. https://bynder.com/static/3.0/img/favicon-black.ico @@ -16,7 +16,7 @@ true BynderDevops https://github.com/Bynder/bynder-c-sharp-sdk - Adds the SDK's name and version to each request in the User-Agent header, to enable better insight in the Bynder API usage. + Performance and stability improvements. The main goal of this SDK is to speed up the integration of Bynder customers who use C# making it easier to connect to the Bynder API (http://docs.bynder.apiary.io/) and executing requests on it. Bynder API C# SDK Bynder.Sdk From c46cdf6f34a19b9a896ac2532f80fc9db74f8248 Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 11:04:29 +0100 Subject: [PATCH 5/9] Simplified code --- .../Api/RequestSender/ApiRequestSenderTest.cs | 105 ++++++------------ 1 file changed, 36 insertions(+), 69 deletions(-) diff --git a/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs b/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs index daf97a4..d5bf8ec 100644 --- a/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs +++ b/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs @@ -3,7 +3,7 @@ using System; using System.Collections.Generic; -using System.Linq; +using System.Net; using System.Net.Http; using System.Text; using System.Threading.Tasks; @@ -40,21 +40,12 @@ public ApiRequestSenderTest() } [Fact] - public async Task WhenErrorResponseThenReturnDefaultObject() + public async Task WhenErrorResponseThenThrowHttpRequestException() { - var expectedResponse = new HttpResponseMessage - { - StatusCode = System.Net.HttpStatusCode.InternalServerError, - Content = new StringContent( - JsonConvert.SerializeObject(_expectedResponseBody), - Encoding.UTF8, - "application/json" - ) - }; var doRequest = SendRequestAsync>( hasValidCredentials: true, httpMethod: HttpMethod.Post, - expectedResponse + CreateResponse(HttpStatusCode.InternalServerError) ); await Assert.ThrowsAsync(() => doRequest); @@ -74,7 +65,7 @@ public async Task WhenEmptyResponseThenReturnDefaultObject() var responseBody = await SendRequestAsync>( hasValidCredentials: true, httpMethod: HttpMethod.Post, - System.Net.HttpStatusCode.OK + CreateResponse(addContent: false) ); Assert.Equal(default(IList), responseBody); @@ -92,11 +83,9 @@ public async Task WhenEmptyResponseThenReturnDefaultObject() [Fact] public async Task WhenRequestIsGetThenParametersAreAddedToUrl() { - var responseBody = await SendRequestAsync( + var responseBody = await SendRequestAsync>( hasValidCredentials: true, - httpMethod: HttpMethod.Get, - System.Net.HttpStatusCode.OK, - _expectedResponseBody + httpMethod: HttpMethod.Get ); Assert.Equal(_expectedResponseBody, responseBody); @@ -114,11 +103,9 @@ public async Task WhenRequestIsGetThenParametersAreAddedToUrl() [Fact] public async Task WhenRequestIsPostThenParametersAreAddedToContent() { - var responseBody = await SendRequestAsync( + var responseBody = await SendRequestAsync>( hasValidCredentials: true, - httpMethod: HttpMethod.Post, - System.Net.HttpStatusCode.OK, - _expectedResponseBody + httpMethod: HttpMethod.Post ); Assert.Equal(_expectedResponseBody, responseBody); @@ -136,11 +123,9 @@ public async Task WhenRequestIsPostThenParametersAreAddedToContent() [Fact] public async Task WhenCredentialInvalidThenTokenIsRefreshed() { - var responseBody = await SendRequestAsync( + var responseBody = await SendRequestAsync>( hasValidCredentials: false, - httpMethod: HttpMethod.Get, - System.Net.HttpStatusCode.OK, - _expectedResponseBody + httpMethod: HttpMethod.Get ); Assert.Equal(_expectedResponseBody, responseBody); @@ -167,44 +152,14 @@ public async Task WhenCredentialInvalidThenTokenIsRefreshed() private async Task SendRequestAsync( bool hasValidCredentials, HttpMethod httpMethod, - System.Net.HttpStatusCode responseStatus) - { - return await SendRequestAsync( - hasValidCredentials, - httpMethod, - new HttpResponseMessage - { - StatusCode = responseStatus - } - ); - } - - private async Task SendRequestAsync( - bool hasValidCredentials, - HttpMethod httpMethod, - System.Net.HttpStatusCode responseStatus, - T responseBody) + HttpResponseMessage response = null) { - return await SendRequestAsync( - hasValidCredentials, - httpMethod, - new HttpResponseMessage - { - StatusCode = responseStatus, - Content = new StringContent( - JsonConvert.SerializeObject(responseBody), - Encoding.UTF8, - "application/json" - ) - } - ); - } + if (response == null) + { + response = CreateResponse(); + } - private async Task SendRequestAsync( - bool hasValidCredentials, - HttpMethod httpMethod, - HttpResponseMessage response) - { + // Setup mock for API requests var httpSenderMockSetup = _httpSenderMock .Setup(sender => sender.SendHttpRequest( It.Is(req => req.RequestUri.PathAndQuery != ApiRequestSender.TokenPath) @@ -218,16 +173,12 @@ private async Task SendRequestAsync( httpSenderMockSetup.ThrowsAsync(new HttpRequestException("")); } + // Setup mock for OAuth token requests _httpSenderMock .Setup(sender => sender.SendHttpRequest( It.Is(req => req.RequestUri.PathAndQuery == ApiRequestSender.TokenPath) )) - .Returns(Task.FromResult( - new HttpResponseMessage - { - StatusCode = System.Net.HttpStatusCode.OK - } - )); + .Returns(Task.FromResult(CreateResponse(addContent: false))); return await CreateApiRequestSender(hasValidCredentials).SendRequestAsync( new ApiRequest @@ -239,6 +190,22 @@ private async Task SendRequestAsync( ); } + private HttpResponseMessage CreateResponse( + HttpStatusCode statusCode = HttpStatusCode.OK, + bool addContent = true) + { + var response = new HttpResponseMessage(statusCode); + if (addContent) + { + response.Content = new StringContent( + JsonConvert.SerializeObject(_expectedResponseBody), + Encoding.UTF8, + "application/json" + ); + } + return response; + } + private IApiRequestSender CreateApiRequestSender(bool hasValidCredentials) { return new ApiRequestSender( @@ -251,9 +218,9 @@ private IApiRequestSender CreateApiRequestSender(bool hasValidCredentials) ); } - private ICredentials GetCredentials(bool isValid) + private Sdk.Settings.ICredentials GetCredentials(bool isValid) { - var credentialsMock = new Mock(); + var credentialsMock = new Mock(); credentialsMock .Setup(mock => mock.AreValid()) From a991943865cbb0d6e9597fb49b3ca1d51c5a9df4 Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 12:38:55 +0100 Subject: [PATCH 6/9] Added issue to release notes --- Bynder/Sdk/Bynder.Sdk.csproj | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Bynder/Sdk/Bynder.Sdk.csproj b/Bynder/Sdk/Bynder.Sdk.csproj index a34be52..5f00b1b 100644 --- a/Bynder/Sdk/Bynder.Sdk.csproj +++ b/Bynder/Sdk/Bynder.Sdk.csproj @@ -16,7 +16,8 @@ true BynderDevops https://github.com/Bynder/bynder-c-sharp-sdk - Performance and stability improvements. + - Performance and stability improvements. +- Fixed error with deserialization of ModifyMedia response The main goal of this SDK is to speed up the integration of Bynder customers who use C# making it easier to connect to the Bynder API (http://docs.bynder.apiary.io/) and executing requests on it. Bynder API C# SDK Bynder.Sdk From f75bba1a4699001c93b07ae77b1ddca21fcea505 Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 13:04:51 +0100 Subject: [PATCH 7/9] Fixed issue with api param conversion. --- Bynder/Sdk/Query/Decoder/IParameterDecoder.cs | 27 ------------------- Bynder/Sdk/Query/Decoder/QueryDecoder.cs | 4 +-- Bynder/Test/Query/Decoder/QueryDecoderTest.cs | 11 ++++---- 3 files changed, 8 insertions(+), 34 deletions(-) delete mode 100644 Bynder/Sdk/Query/Decoder/IParameterDecoder.cs diff --git a/Bynder/Sdk/Query/Decoder/IParameterDecoder.cs b/Bynder/Sdk/Query/Decoder/IParameterDecoder.cs deleted file mode 100644 index 3b90f15..0000000 --- a/Bynder/Sdk/Query/Decoder/IParameterDecoder.cs +++ /dev/null @@ -1,27 +0,0 @@ -// Copyright (c) Bynder. All rights reserved. -// Licensed under the MIT License. See LICENSE file in the project root for full license information. - -using System; - -namespace Bynder.Sdk.Query.Decoder -{ - /// - /// Decoder interface to use in queries to convert from one specific type to string. - /// - public interface IParameterDecoder - { - /// - /// Checks if the converter can convert a specific type. - /// - /// Type to convert from - /// true if it can convert the type - bool CanConvert(Type typeToConvert); - - /// - /// Converts the value to string. - /// - /// value to be converted - /// converted string value - string Convert(object value); - } -} \ No newline at end of file diff --git a/Bynder/Sdk/Query/Decoder/QueryDecoder.cs b/Bynder/Sdk/Query/Decoder/QueryDecoder.cs index 7ffd5d4..a84aec5 100644 --- a/Bynder/Sdk/Query/Decoder/QueryDecoder.cs +++ b/Bynder/Sdk/Query/Decoder/QueryDecoder.cs @@ -3,8 +3,8 @@ using System; using System.Collections.Generic; -using System.Collections.Specialized; using System.Reflection; +using Bynder.Sdk.Api.Converters; namespace Bynder.Sdk.Query.Decoder { @@ -79,7 +79,7 @@ private string ConvertPropertyValue(ApiField apiField, Type propertyType, object bool isConverted = false; if (apiField.Converter != null) { - IParameterDecoder converter = Activator.CreateInstance(apiField.Converter) as IParameterDecoder; + ITypeToStringConverter converter = Activator.CreateInstance(apiField.Converter) as ITypeToStringConverter; if (converter != null && converter.CanConvert(propertyType)) diff --git a/Bynder/Test/Query/Decoder/QueryDecoderTest.cs b/Bynder/Test/Query/Decoder/QueryDecoderTest.cs index 03b7ce7..fa9e5b0 100644 --- a/Bynder/Test/Query/Decoder/QueryDecoderTest.cs +++ b/Bynder/Test/Query/Decoder/QueryDecoderTest.cs @@ -2,6 +2,7 @@ // Licensed under the MIT License. See LICENSE file in the project root for full license information. using System; +using Bynder.Sdk.Api.Converters; using Bynder.Sdk.Query.Decoder; using Xunit; @@ -73,13 +74,13 @@ private class StubQuery /// /// Stub converter only used for testing purposes. /// - private class StubDecoder : IParameterDecoder + private class StubDecoder : ITypeToStringConverter { /// /// Check . /// - /// Check - /// Check + /// Check + /// Check public bool CanConvert(Type typeToConvert) { return true; @@ -88,8 +89,8 @@ public bool CanConvert(Type typeToConvert) /// /// Check . /// - /// Check - /// Check + /// Check + /// Check public string Convert(object value) { return "Converted"; From adc6bad605a0a2d5bab931f9e0fd056968cb5b00 Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 15:36:21 +0100 Subject: [PATCH 8/9] Moved RefreshToken logic to OauthService --- .../Sdk/Api/RequestSender/ApiRequestSender.cs | 54 +++----- Bynder/Sdk/Api/Requests/OAuthRequest.cs | 3 + Bynder/Sdk/Service/BynderClient.cs | 2 +- Bynder/Sdk/Service/OAuth/IOAuthService.cs | 6 + Bynder/Sdk/Service/OAuth/OAuthService.cs | 36 ++++- .../Api/RequestSender/ApiRequestSenderTest.cs | 25 +--- Bynder/Test/Service/OAuth/OAuthServiceTest.cs | 124 +++++++++++++----- 7 files changed, 153 insertions(+), 97 deletions(-) diff --git a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs index 2e0327b..13d1b00 100644 --- a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs +++ b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs @@ -8,9 +8,8 @@ using System.Threading; using System.Threading.Tasks; using Bynder.Sdk.Api.Requests; -using Bynder.Sdk.Model; -using Bynder.Sdk.Query; using Bynder.Sdk.Query.Decoder; +using Bynder.Sdk.Service.OAuth; using Bynder.Sdk.Settings; using Newtonsoft.Json; @@ -24,21 +23,21 @@ internal class ApiRequestSender : IApiRequestSender private readonly Configuration _configuration; private readonly QueryDecoder _queryDecoder = new QueryDecoder(); private readonly ICredentials _credentials; + private readonly IOAuthService _oauthService; private SemaphoreSlim _semaphore = new SemaphoreSlim(1, 1); private IHttpRequestSender _httpSender; - public const string TokenPath = "/v6/authentication/oauth2/token"; - /// /// Initializes a new instance of the class. /// /// Configuration. /// Credentials to use in authorized requests and to refresh tokens /// HTTP instance to send API requests - internal ApiRequestSender(Configuration configuration, ICredentials credentials, IHttpRequestSender httpSender) + internal ApiRequestSender(Configuration configuration, ICredentials credentials, IOAuthService oauthService, IHttpRequestSender httpSender) { _configuration = configuration; _credentials = credentials; + _oauthService = oauthService; _httpSender = httpSender; } @@ -48,9 +47,9 @@ internal ApiRequestSender(Configuration configuration, ICredentials credentials, /// The instance. /// Configuration. /// Credentials. - public static IApiRequestSender Create(Configuration configuration, ICredentials credentials) + public static IApiRequestSender Create(Configuration configuration, ICredentials credentials, IOAuthService oauthService) { - return new ApiRequestSender(configuration, credentials, new HttpRequestSender()); + return new ApiRequestSender(configuration, credentials, oauthService, new HttpRequestSender()); } /// @@ -94,11 +93,10 @@ public async Task SendRequestAsync(Request request) private async Task CreateHttpRequestAsync(Request request) { - var parameters = _queryDecoder.GetParameters(request.Query); var httpRequestMessage = HttpRequestMessageFactory.Create( _configuration.BaseUrl.ToString(), request.HTTPMethod, - parameters, + _queryDecoder.GetParameters(request.Query), request.Path ); @@ -106,7 +104,16 @@ private async Task CreateHttpRequestAsync(Request req { if (!_credentials.AreValid()) { - _credentials.Update(await RefreshToken().ConfigureAwait(false)); + // Get a refesh token when the credentials are no longer valid + await _semaphore.WaitAsync().ConfigureAwait(false); + try + { + await _oauthService.GetRefreshTokenAsync().ConfigureAwait(false); + } + finally + { + _semaphore.Release(); + } } httpRequestMessage.Headers.Authorization = new AuthenticationHeaderValue( @@ -118,33 +125,6 @@ private async Task CreateHttpRequestAsync(Request req return await _httpSender.SendHttpRequest(httpRequestMessage).ConfigureAwait(false); } - private async Task RefreshToken() - { - await _semaphore.WaitAsync().ConfigureAwait(false); - try - { - return await SendRequestAsync( - new OAuthRequest - { - Authenticated = false, - Query = new TokenQuery - { - ClientId = _configuration.ClientId, - ClientSecret = _configuration.ClientSecret, - RefreshToken = _credentials.RefreshToken, - GrantType = "refresh_token" - }, - Path = TokenPath, - HTTPMethod = HttpMethod.Post - } - ).ConfigureAwait(false); - } - finally - { - _semaphore.Release(); - } - } - private static class HttpRequestMessageFactory { internal static HttpRequestMessage Create( diff --git a/Bynder/Sdk/Api/Requests/OAuthRequest.cs b/Bynder/Sdk/Api/Requests/OAuthRequest.cs index 8ac502d..2bbb0e8 100644 --- a/Bynder/Sdk/Api/Requests/OAuthRequest.cs +++ b/Bynder/Sdk/Api/Requests/OAuthRequest.cs @@ -9,5 +9,8 @@ namespace Bynder.Sdk.Api.Requests /// Type to which the response will be deserialized internal class OAuthRequest : Request { + public OAuthRequest() { + Authenticated = false; + } } } diff --git a/Bynder/Sdk/Service/BynderClient.cs b/Bynder/Sdk/Service/BynderClient.cs index a3b59cf..2d5c7cd 100644 --- a/Bynder/Sdk/Service/BynderClient.cs +++ b/Bynder/Sdk/Service/BynderClient.cs @@ -41,7 +41,7 @@ public BynderClient(Configuration configuration) { _credentials = new Credentials(configuration.Token); } - _requestSender = ApiRequestSender.Create(_configuration, _credentials); + _requestSender = ApiRequestSender.Create(_configuration, _credentials, _oauthService); } /// diff --git a/Bynder/Sdk/Service/OAuth/IOAuthService.cs b/Bynder/Sdk/Service/OAuth/IOAuthService.cs index 4a60792..7c118ba 100644 --- a/Bynder/Sdk/Service/OAuth/IOAuthService.cs +++ b/Bynder/Sdk/Service/OAuth/IOAuthService.cs @@ -25,5 +25,11 @@ public interface IOAuthService /// Code received after the redirect /// The authorization scopes Task GetAccessTokenAsync(string code, string scopes); + + /// + /// Gets a refresh token. + /// + /// The task to get the refresh token + Task GetRefreshTokenAsync(); } } diff --git a/Bynder/Sdk/Service/OAuth/OAuthService.cs b/Bynder/Sdk/Service/OAuth/OAuthService.cs index b79c284..c0b2bed 100644 --- a/Bynder/Sdk/Service/OAuth/OAuthService.cs +++ b/Bynder/Sdk/Service/OAuth/OAuthService.cs @@ -19,6 +19,9 @@ internal class OAuthService : IOAuthService /// private IApiRequestSender _requestSender; + public const string AuthPath = "/v6/authentication/oauth2/auth"; + public const string TokenPath = "/v6/authentication/oauth2/token"; + /// /// Initializes a new instance of the class /// @@ -53,7 +56,7 @@ public string GetAuthorisationUrl(string state, string scopes) }; var builder = new UriBuilder(_configuration.BaseUrl); - builder.Path = "/v6/authentication/oauth2/auth"; + builder.Path = AuthPath; builder.Query = Utils.Url.ConvertToQuery(authoriseParams); @@ -73,7 +76,7 @@ public async Task GetAccessTokenAsync(string code, string scopes) throw new ArgumentNullException(code); } - TokenQuery query = new TokenQuery + var query = new TokenQuery { ClientId = _configuration.ClientId, ClientSecret = _configuration.ClientSecret, @@ -86,14 +89,39 @@ public async Task GetAccessTokenAsync(string code, string scopes) var request = new OAuthRequest { Query = query, - Path = "/v6/authentication/oauth2/token", + Path = TokenPath, HTTPMethod = HttpMethod.Post, - Authenticated = false }; var token = await _requestSender.SendRequestAsync(request).ConfigureAwait(false); token.SetAccessTokenExpiration(); _credentials.Update(token); } + + /// + /// Check . + /// + /// Check . + public async Task GetRefreshTokenAsync() + { + var query = new TokenQuery + { + ClientId = _configuration.ClientId, + ClientSecret = _configuration.ClientSecret, + RefreshToken = _credentials.RefreshToken, + GrantType = "refresh_token" + }; + + var request = new OAuthRequest + { + Query = query, + Path = TokenPath, + HTTPMethod = HttpMethod.Post, + }; + + var token = await _requestSender.SendRequestAsync(request).ConfigureAwait(false); + _credentials.Update(token); + } + } } diff --git a/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs b/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs index d5bf8ec..ec95db0 100644 --- a/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs +++ b/Bynder/Test/Api/RequestSender/ApiRequestSenderTest.cs @@ -10,6 +10,7 @@ using Bynder.Sdk.Api.Requests; using Bynder.Sdk.Api.RequestSender; using Bynder.Sdk.Query.Decoder; +using Bynder.Sdk.Service.OAuth; using Bynder.Sdk.Settings; using Moq; using Newtonsoft.Json; @@ -25,12 +26,14 @@ public class ApiRequestSenderTest private const string _queryValue = "some_query_value"; private const string _queryString = "Item1=" + _queryValue; + private Mock _oAuthServiceMock; private Mock _httpSenderMock; private StubQuery _query; private IList _expectedResponseBody; public ApiRequestSenderTest() { + _oAuthServiceMock = new Mock(); _httpSenderMock = new Mock(); _query = new StubQuery { @@ -130,15 +133,8 @@ public async Task WhenCredentialInvalidThenTokenIsRefreshed() Assert.Equal(_expectedResponseBody, responseBody); - // Check Refresh token request - _httpSenderMock.Verify(sender => sender.SendHttpRequest( - It.Is( - req => req.RequestUri.PathAndQuery == ApiRequestSender.TokenPath - && req.Method == HttpMethod.Post - ) - ), Times.Once); + _oAuthServiceMock.Verify(oAuthService => oAuthService.GetRefreshTokenAsync(), Times.Once); - // Check API request _httpSenderMock.Verify(sender => sender.SendHttpRequest( It.Is( req => req.RequestUri.PathAndQuery.Contains(_path) @@ -159,11 +155,8 @@ private async Task SendRequestAsync( response = CreateResponse(); } - // Setup mock for API requests var httpSenderMockSetup = _httpSenderMock - .Setup(sender => sender.SendHttpRequest( - It.Is(req => req.RequestUri.PathAndQuery != ApiRequestSender.TokenPath) - )); + .Setup(sender => sender.SendHttpRequest(It.IsAny())); if (response.IsSuccessStatusCode) { httpSenderMockSetup.Returns(Task.FromResult(response)); @@ -173,13 +166,6 @@ private async Task SendRequestAsync( httpSenderMockSetup.ThrowsAsync(new HttpRequestException("")); } - // Setup mock for OAuth token requests - _httpSenderMock - .Setup(sender => sender.SendHttpRequest( - It.Is(req => req.RequestUri.PathAndQuery == ApiRequestSender.TokenPath) - )) - .Returns(Task.FromResult(CreateResponse(addContent: false))); - return await CreateApiRequestSender(hasValidCredentials).SendRequestAsync( new ApiRequest { @@ -214,6 +200,7 @@ private IApiRequestSender CreateApiRequestSender(bool hasValidCredentials) BaseUrl = new Uri("https://example.bynder.com"), }, GetCredentials(hasValidCredentials), + _oAuthServiceMock.Object, _httpSenderMock.Object ); } diff --git a/Bynder/Test/Service/OAuth/OAuthServiceTest.cs b/Bynder/Test/Service/OAuth/OAuthServiceTest.cs index 9925e09..f0bbba2 100644 --- a/Bynder/Test/Service/OAuth/OAuthServiceTest.cs +++ b/Bynder/Test/Service/OAuth/OAuthServiceTest.cs @@ -7,6 +7,7 @@ using Bynder.Sdk.Api.Requests; using Bynder.Sdk.Api.RequestSender; using Bynder.Sdk.Model; +using Bynder.Sdk.Query; using Bynder.Sdk.Service.OAuth; using Bynder.Sdk.Settings; using Moq; @@ -16,56 +17,107 @@ namespace Bynder.Test.Service.OAuth { public class OAuthServiceTest { + private const string _clientId = "clientId"; + private const string _clientSecret = "clientSecret"; + private const string _baseUrl = "https://example.bynder.com"; + private const string _redirectUrl = "https://redirect.bynder.com"; + private const string _redirectUrlEncoded = "https%3A%2F%2Fredirect.bynder.com"; + private const string _refreshToken = "refreshToken"; + private const string _state = "state"; + private const string _scope = "offline"; + private const string _code = "code"; + + private Token _token; + private Mock _credentialsMock; + private Mock _apiRequestSenderMock; + private OAuthService _oauthService; + + public OAuthServiceTest() { + _token = new Token(); + + _credentialsMock = new Mock(); + _credentialsMock + .SetupGet(credentials => credentials.RefreshToken) + .Returns(_refreshToken); + + _apiRequestSenderMock = new Mock(); + _apiRequestSenderMock + .Setup(sender => sender.SendRequestAsync(It.IsAny>())) + .Returns(Task.FromResult(_token)); + + _oauthService = new OAuthService( + new Configuration + { + BaseUrl = new Uri(_baseUrl), + ClientId = _clientId, + ClientSecret = _clientSecret, + RedirectUri = _redirectUrl, + }, + _credentialsMock.Object, + _apiRequestSenderMock.Object + ); + + } + [Fact] public void GetAuthorisationUrlReturnsCorrectUrl() { - - OAuthService oauth = new OAuthService( - new Configuration { - BaseUrl = new Uri("https://example.bynder.com"), - ClientId = "clientId", - ClientSecret = "clientSecret", - RedirectUri = "https://redirect.bynder.com" - }, null, null); - - var authorisationUrl = oauth.GetAuthorisationUrl("state example", "offline"); - - Assert.Equal("https://example.bynder.com:443/v6/authentication/oauth2/auth?client_id=clientId&redirect_uri=https%3A%2F%2Fredirect.bynder.com&response_type=code&scope=offline&state=state%20example", - authorisationUrl); + var authorisationUrl = _oauthService.GetAuthorisationUrl(_state, _scope); + + Assert.Equal( + $"{_baseUrl}:443{OAuthService.AuthPath}?client_id={_clientId}&redirect_uri={_redirectUrlEncoded}&response_type={_code}&scope={_scope}&state={_state}", + authorisationUrl + ); } [Fact] public async Task GetAccessTokenCallsRequestAsyncAndUpdates() { - var apiRequestSender = new Mock(); - var result = new Token(); - var credentials = new Mock(); - apiRequestSender.Setup(sender => sender.SendRequestAsync(It.IsAny>())) - .Returns(Task.FromResult(result)); - var oauthService = new OAuthService( - new Configuration { - BaseUrl = new Uri("https://example.bynder.com"), - ClientId = "clientId", - ClientSecret = "clientSecret", - RedirectUri = "https://redirect.bynder.com" - }, - credentials.Object, - apiRequestSender.Object); - - await oauthService.GetAccessTokenAsync("code", "offline").ConfigureAwait(false); + await _oauthService.GetAccessTokenAsync(_code, _scope).ConfigureAwait(false); + + _apiRequestSenderMock.Verify(sender => sender.SendRequestAsync( + It.Is>( + req => req.Path == OAuthService.TokenPath + && req.HTTPMethod == HttpMethod.Post + && !req.Authenticated + && (req.Query as TokenQuery).ClientId == _clientId + && (req.Query as TokenQuery).ClientSecret == _clientSecret + && (req.Query as TokenQuery).RedirectUri == _redirectUrl + && (req.Query as TokenQuery).GrantType == "authorization_code" + && (req.Query as TokenQuery).Code == _code + && (req.Query as TokenQuery).Scopes == _scope + ) + ), Times.Once); + + _credentialsMock.Verify(cred => cred.Update( + It.Is( + token => token == _token + ) + ), Times.Once); + } + + [Fact] + public async Task GetRefreshTokenCallsRequestAsyncAndUpdates() + { + await _oauthService.GetRefreshTokenAsync().ConfigureAwait(false); - apiRequestSender.Verify(sender => sender.SendRequestAsync( + _apiRequestSenderMock.Verify(sender => sender.SendRequestAsync( It.Is>( - req => req.Path == "/v6/authentication/oauth2/token" + req => req.Path == OAuthService.TokenPath && req.HTTPMethod == HttpMethod.Post && !req.Authenticated - && req.Query != null - ))); + && (req.Query as TokenQuery).ClientId == _clientId + && (req.Query as TokenQuery).ClientSecret == _clientSecret + && (req.Query as TokenQuery).RefreshToken == _refreshToken + && (req.Query as TokenQuery).GrantType == "refresh_token" + ) + ), Times.Once); - credentials.Verify(cred => cred.Update( + _credentialsMock.Verify(cred => cred.Update( It.Is( - token => token == result - ))); + token => token == _token + ) + ), Times.Once); } } } From d7da24f089f367be4c7384ba460bac76ed65fc7f Mon Sep 17 00:00:00 2001 From: Huib Piguillet Date: Thu, 19 Nov 2020 16:29:30 +0100 Subject: [PATCH 9/9] Missing docstring params --- Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs index 13d1b00..e44e755 100644 --- a/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs +++ b/Bynder/Sdk/Api/RequestSender/ApiRequestSender.cs @@ -32,6 +32,7 @@ internal class ApiRequestSender : IApiRequestSender /// /// Configuration. /// Credentials to use in authorized requests and to refresh tokens + /// OAuthService. /// HTTP instance to send API requests internal ApiRequestSender(Configuration configuration, ICredentials credentials, IOAuthService oauthService, IHttpRequestSender httpSender) { @@ -47,6 +48,7 @@ internal ApiRequestSender(Configuration configuration, ICredentials credentials, /// The instance. /// Configuration. /// Credentials. + /// OAuthService. public static IApiRequestSender Create(Configuration configuration, ICredentials credentials, IOAuthService oauthService) { return new ApiRequestSender(configuration, credentials, oauthService, new HttpRequestSender());