From 6d0120d564183984d77574921b401127934e43c7 Mon Sep 17 00:00:00 2001 From: Jkorf Date: Wed, 1 Dec 2021 16:26:34 +0100 Subject: [PATCH] Comments, fix test --- CryptoExchange.Net.UnitTests/OptionsTests.cs | 8 +++ .../RestClientTests.cs | 12 ++--- .../TestImplementations/TestRestClient.cs | 1 + CryptoExchange.Net/Attributes/MapAttribute.cs | 13 ++++- CryptoExchange.Net/Clients/BaseApiClient.cs | 22 ++++++-- CryptoExchange.Net/Clients/BaseClient.cs | 4 +- CryptoExchange.Net/Clients/BaseRestClient.cs | 3 +- .../Clients/BaseSocketClient.cs | 12 +++-- CryptoExchange.Net/Clients/RestApiClient.cs | 24 ++++----- CryptoExchange.Net/Clients/SocketApiClient.cs | 26 ++++----- .../Converters/DateTimeConverter.cs | 6 +-- .../Converters/EnumConverter.cs | 16 +++++- CryptoExchange.Net/Objects/Options.cs | 53 +++++++++++++++---- 13 files changed, 134 insertions(+), 66 deletions(-) diff --git a/CryptoExchange.Net.UnitTests/OptionsTests.cs b/CryptoExchange.Net.UnitTests/OptionsTests.cs index a8cfa05..c56ede9 100644 --- a/CryptoExchange.Net.UnitTests/OptionsTests.cs +++ b/CryptoExchange.Net.UnitTests/OptionsTests.cs @@ -14,6 +14,14 @@ namespace CryptoExchange.Net.UnitTests [TestFixture()] public class OptionsTests { + [TearDown] + public void Init() + { + TestClientOptions.Default = new TestClientOptions + { + }; + } + [TestCase(null, null)] [TestCase("", "")] [TestCase("test", null)] diff --git a/CryptoExchange.Net.UnitTests/RestClientTests.cs b/CryptoExchange.Net.UnitTests/RestClientTests.cs index 9107ed0..44d225d 100644 --- a/CryptoExchange.Net.UnitTests/RestClientTests.cs +++ b/CryptoExchange.Net.UnitTests/RestClientTests.cs @@ -51,14 +51,14 @@ namespace CryptoExchange.Net.UnitTests } [TestCase] - public void ReceivingErrorCode_Should_ResultInError() + public async Task ReceivingErrorCode_Should_ResultInError() { // arrange var client = new TestRestClient(); client.SetErrorWithoutResponse(System.Net.HttpStatusCode.BadRequest, "Invalid request"); // act - var result = client.Request().Result; + var result = await client.Request(); // assert Assert.IsFalse(result.Success); @@ -66,14 +66,14 @@ namespace CryptoExchange.Net.UnitTests } [TestCase] - public void ReceivingErrorAndNotParsingError_Should_ResultInFlatError() + public async Task ReceivingErrorAndNotParsingError_Should_ResultInFlatError() { // arrange var client = new TestRestClient(); client.SetErrorWithResponse("{\"errorMessage\": \"Invalid request\", \"errorCode\": 123}", System.Net.HttpStatusCode.BadRequest); // act - var result = client.Request().Result; + var result = await client.Request(); // assert Assert.IsFalse(result.Success); @@ -84,14 +84,14 @@ namespace CryptoExchange.Net.UnitTests } [TestCase] - public void ReceivingErrorAndParsingError_Should_ResultInParsedError() + public async Task ReceivingErrorAndParsingError_Should_ResultInParsedError() { // arrange var client = new ParseErrorTestRestClient(); client.SetErrorWithResponse("{\"errorMessage\": \"Invalid request\", \"errorCode\": 123}", System.Net.HttpStatusCode.BadRequest); // act - var result = client.Request().Result; + var result = await client.Request(); // assert Assert.IsFalse(result.Success); diff --git a/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs b/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs index c9a1263..3fffc33 100644 --- a/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs +++ b/CryptoExchange.Net.UnitTests/TestImplementations/TestRestClient.cs @@ -72,6 +72,7 @@ namespace CryptoExchange.Net.UnitTests.TestImplementations typeof(HttpRequestException).GetField("_message", BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance).SetValue(we, message); var request = new Mock(); + request.Setup(c => c.GetHeaders()).Returns(new Dictionary>()); request.Setup(c => c.GetResponseAsync(It.IsAny())).Throws(we); var factory = Mock.Get(RequestFactory); diff --git a/CryptoExchange.Net/Attributes/MapAttribute.cs b/CryptoExchange.Net/Attributes/MapAttribute.cs index 27f553a..1cdc145 100644 --- a/CryptoExchange.Net/Attributes/MapAttribute.cs +++ b/CryptoExchange.Net/Attributes/MapAttribute.cs @@ -1,12 +1,21 @@ using System; -using System.Collections.Generic; -using System.Text; namespace CryptoExchange.Net.Attributes { + /// + /// Map a enum entry to string values + /// public class MapAttribute : Attribute { + /// + /// Values mapping to the enum entry + /// public string[] Values { get; set; } + + /// + /// ctor + /// + /// public MapAttribute(params string[] maps) { Values = maps; diff --git a/CryptoExchange.Net/Clients/BaseApiClient.cs b/CryptoExchange.Net/Clients/BaseApiClient.cs index 58b8f78..cd4a29b 100644 --- a/CryptoExchange.Net/Clients/BaseApiClient.cs +++ b/CryptoExchange.Net/Clients/BaseApiClient.cs @@ -19,12 +19,15 @@ using Newtonsoft.Json.Linq; namespace CryptoExchange.Net { /// - /// Base rest client + /// Base API for all API clients /// public abstract class BaseApiClient: IDisposable { - private ApiCredentials? _apiCredentials; - private AuthenticationProvider _authenticationProvider; + private readonly ApiCredentials? _apiCredentials; + private AuthenticationProvider? _authenticationProvider; + /// + /// The authentication provider for this API client. (null if no credentials are set) + /// public AuthenticationProvider? AuthenticationProvider { get @@ -36,14 +39,27 @@ namespace CryptoExchange.Net } } + /// + /// The base address for this API client + /// internal protected string BaseAddress { get; } + /// + /// ctor + /// + /// Client options + /// Api client options public BaseApiClient(BaseClientOptions options, ApiClientOptions apiOptions) { _apiCredentials = apiOptions.ApiCredentials ?? options.ApiCredentials; BaseAddress = apiOptions.BaseAddress; } + /// + /// Create an AuthenticationProvider implementation instance based on the provided credentials + /// + /// + /// public abstract AuthenticationProvider CreateAuthenticationProvider(ApiCredentials credentials); /// diff --git a/CryptoExchange.Net/Clients/BaseClient.cs b/CryptoExchange.Net/Clients/BaseClient.cs index bfb4b0e..e923596 100644 --- a/CryptoExchange.Net/Clients/BaseClient.cs +++ b/CryptoExchange.Net/Clients/BaseClient.cs @@ -238,7 +238,7 @@ namespace CryptoExchange.Net } } - private async Task ReadStreamAsync(Stream stream) + private static async Task ReadStreamAsync(Stream stream) { using var reader = new StreamReader(stream, Encoding.UTF8, false, 512, true); return await reader.ReadToEndAsync().ConfigureAwait(false); @@ -248,7 +248,7 @@ namespace CryptoExchange.Net /// Generate a new unique id. The id is staticly stored so it is guarenteed to be unique across different client instances /// /// - protected int NextId() + protected static int NextId() { lock (idLock) { diff --git a/CryptoExchange.Net/Clients/BaseRestClient.cs b/CryptoExchange.Net/Clients/BaseRestClient.cs index 2b995f3..1d2fbce 100644 --- a/CryptoExchange.Net/Clients/BaseRestClient.cs +++ b/CryptoExchange.Net/Clients/BaseRestClient.cs @@ -8,7 +8,6 @@ using System.Net.Http; using System.Threading; using System.Threading.Tasks; using System.Web; -using CryptoExchange.Net.Authentication; using CryptoExchange.Net.Interfaces; using CryptoExchange.Net.Objects; using CryptoExchange.Net.Requests; @@ -91,6 +90,7 @@ namespace CryptoExchange.Net /// Execute a request to the uri and deserialize the response into the provided type parameter /// /// The type to deserialize into + /// The API client the request is for /// The uri to send the request to /// The method of the request /// Cancellation token @@ -257,6 +257,7 @@ namespace CryptoExchange.Net /// /// Creates a request object /// + /// The API client the request is for /// The uri to send the request to /// The method of the request /// The parameters of the request diff --git a/CryptoExchange.Net/Clients/BaseSocketClient.cs b/CryptoExchange.Net/Clients/BaseSocketClient.cs index 3028a0a..36a79e0 100644 --- a/CryptoExchange.Net/Clients/BaseSocketClient.cs +++ b/CryptoExchange.Net/Clients/BaseSocketClient.cs @@ -6,7 +6,6 @@ using System.Linq; using System.Net.WebSockets; using System.Threading; using System.Threading.Tasks; -using CryptoExchange.Net.Authentication; using CryptoExchange.Net.Interfaces; using CryptoExchange.Net.Objects; using CryptoExchange.Net.Sockets; @@ -30,11 +29,11 @@ namespace CryptoExchange.Net /// /// List of socket connections currently connecting/connected /// - protected internal ConcurrentDictionary sockets = new ConcurrentDictionary(); + protected internal ConcurrentDictionary sockets = new(); /// /// Semaphore used while creating sockets /// - protected internal readonly SemaphoreSlim semaphoreSlim = new SemaphoreSlim(1); + protected internal readonly SemaphoreSlim semaphoreSlim = new(1); /// /// The max amount of concurrent socket connections /// @@ -50,7 +49,7 @@ namespace CryptoExchange.Net /// /// Handlers for data from the socket which doesn't need to be forwarded to the caller. Ping or welcome messages for example. /// - protected Dictionary> genericHandlers = new Dictionary>(); + protected Dictionary> genericHandlers = new(); /// /// The task that is sending periodic data on the websocket. Can be used for sending Ping messages every x seconds or similair. Not necesarry. /// @@ -127,6 +126,7 @@ namespace CryptoExchange.Net /// Connect to an url and listen for data on the BaseAddress /// /// The type of the expected data + /// The API client the subscription is for /// The optional request object to send, will be serialized to json /// The identifier to use, necessary if no request object is sent /// If the subscription is to an authenticated endpoint @@ -142,6 +142,7 @@ namespace CryptoExchange.Net /// Connect to an url and listen for data /// /// The type of the expected data + /// The API client the subscription is for /// The URL to connect to /// The optional request object to send, will be serialized to json /// The identifier to use, necessary if no request object is sent @@ -250,6 +251,7 @@ namespace CryptoExchange.Net /// Send a query on a socket connection to the BaseAddress and wait for the response /// /// Expected result type + /// The API client the query is for /// The request to send, will be serialized to json /// If the query is to an authenticated endpoint /// @@ -262,6 +264,7 @@ namespace CryptoExchange.Net /// Send a query on a socket connection and wait for the response /// /// The expected result type + /// The API client the query is for /// The url for the request /// The request to send /// Whether the socket should be authenticated @@ -477,6 +480,7 @@ namespace CryptoExchange.Net /// /// Gets a connection for a new subscription or query. Can be an existing if there are open position or a new one. /// + /// The API client the connection is for /// The address the socket is for /// Whether the socket should be authenticated /// diff --git a/CryptoExchange.Net/Clients/RestApiClient.cs b/CryptoExchange.Net/Clients/RestApiClient.cs index 29e892f..f0e9550 100644 --- a/CryptoExchange.Net/Clients/RestApiClient.cs +++ b/CryptoExchange.Net/Clients/RestApiClient.cs @@ -1,28 +1,17 @@ -using System; using System.Collections.Generic; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.Linq; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; -using System.Web; -using CryptoExchange.Net.Authentication; using CryptoExchange.Net.Interfaces; using CryptoExchange.Net.Objects; -using CryptoExchange.Net.Requests; -using Microsoft.Extensions.Logging; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; namespace CryptoExchange.Net { /// - /// Base rest client + /// Base rest API client for interacting with a REST API /// public abstract class RestApiClient: BaseApiClient { + /// + /// Options for this client + /// internal RestApiClientOptions Options { get; } /// @@ -30,6 +19,11 @@ namespace CryptoExchange.Net /// internal IEnumerable RateLimiters { get; } + /// + /// ctor + /// + /// The base client options + /// The Api client options public RestApiClient(BaseRestClientOptions options, RestApiClientOptions apiOptions): base(options, apiOptions) { Options = apiOptions; diff --git a/CryptoExchange.Net/Clients/SocketApiClient.cs b/CryptoExchange.Net/Clients/SocketApiClient.cs index 5c85547..05fc465 100644 --- a/CryptoExchange.Net/Clients/SocketApiClient.cs +++ b/CryptoExchange.Net/Clients/SocketApiClient.cs @@ -1,30 +1,22 @@ -using System; -using System.Collections.Generic; -using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; -using System.IO; -using System.Linq; -using System.Net.Http; -using System.Threading; -using System.Threading.Tasks; -using System.Web; -using CryptoExchange.Net.Authentication; -using CryptoExchange.Net.Interfaces; using CryptoExchange.Net.Objects; -using CryptoExchange.Net.Requests; -using Microsoft.Extensions.Logging; -using Newtonsoft.Json; -using Newtonsoft.Json.Linq; namespace CryptoExchange.Net { /// - /// Base rest client + /// Base socket API client for interaction with a websocket API /// public abstract class SocketApiClient : BaseApiClient { + /// + /// The options for this client + /// internal ApiClientOptions Options { get; } + /// + /// ctor + /// + /// The base client options + /// The Api client options public SocketApiClient(BaseClientOptions options, ApiClientOptions apiOptions): base(options, apiOptions) { Options = apiOptions; diff --git a/CryptoExchange.Net/Converters/DateTimeConverter.cs b/CryptoExchange.Net/Converters/DateTimeConverter.cs index 4d471e2..1a0d27b 100644 --- a/CryptoExchange.Net/Converters/DateTimeConverter.cs +++ b/CryptoExchange.Net/Converters/DateTimeConverter.cs @@ -6,11 +6,11 @@ using System.Globalization; namespace CryptoExchange.Net.Converters { /// - /// Datetime converter + /// Datetime converter. Supports converting from string/long/double to DateTime and back. Numbers are assumed to be the time since 1970-01-01. /// public class DateTimeConverter: JsonConverter { - private static DateTime _epoch = new DateTime(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); + private static readonly DateTime _epoch = new(1970, 1, 1, 0, 0, 0, DateTimeKind.Utc); private const decimal ticksPerMicrosecond = TimeSpan.TicksPerMillisecond / 1000; private const decimal ticksPerNanosecond = TimeSpan.TicksPerMillisecond / 1000m / 1000; @@ -180,7 +180,7 @@ namespace CryptoExchange.Net.Converters if(datetimeValue == default(DateTime)) writer.WriteValue((DateTime?)null); else - writer.WriteValue((long)Math.Round(((DateTime)value - new DateTime(1970, 1, 1)).TotalMilliseconds)); + writer.WriteValue((long)Math.Round(((DateTime)value! - new DateTime(1970, 1, 1)).TotalMilliseconds)); } } } diff --git a/CryptoExchange.Net/Converters/EnumConverter.cs b/CryptoExchange.Net/Converters/EnumConverter.cs index 9a90c55..0f9be4b 100644 --- a/CryptoExchange.Net/Converters/EnumConverter.cs +++ b/CryptoExchange.Net/Converters/EnumConverter.cs @@ -8,15 +8,20 @@ using System.Linq; namespace CryptoExchange.Net.Converters { + /// + /// Converter for enum values. Enums entries should be noted with a MapAttribute to map the enum value to a string value + /// public class EnumConverter : JsonConverter { - private static ConcurrentDictionary>> _mapping = new ConcurrentDictionary>>(); + private static readonly ConcurrentDictionary>> _mapping = new(); + /// public override bool CanConvert(Type objectType) { return objectType.IsEnum; } + /// public override object? ReadJson(JsonReader reader, Type objectType, object? existingValue, JsonSerializer serializer) { objectType = Nullable.GetUnderlyingType(objectType) ?? objectType; @@ -56,7 +61,7 @@ namespace CryptoExchange.Net.Converters return mapping; } - private bool GetValue(Type objectType, List> enumMapping, string value, out object? result) + private static bool GetValue(Type objectType, List> enumMapping, string value, out object? result) { // Check for exact match first, then if not found fallback to a case insensitive match var mapping = enumMapping.FirstOrDefault(kv => kv.Value.Equals(value, StringComparison.InvariantCulture)); @@ -81,6 +86,12 @@ namespace CryptoExchange.Net.Converters } } + /// + /// Get the string value for an enum value using the MapAttribute mapping. When multiple values are mapped for a enum entry the first value will be returned + /// + /// + /// + /// public static string? GetString(T enumValue) { var objectType = typeof(T); @@ -92,6 +103,7 @@ namespace CryptoExchange.Net.Converters return enumValue == null ? null : (mapping.FirstOrDefault(v => v.Key.Equals(enumValue)).Value ?? enumValue.ToString()); } + /// public override void WriteJson(JsonWriter writer, object? value, JsonSerializer serializer) { var stringValue = GetString(value); diff --git a/CryptoExchange.Net/Objects/Options.cs b/CryptoExchange.Net/Objects/Options.cs index d423a60..ae55a7d 100644 --- a/CryptoExchange.Net/Objects/Options.cs +++ b/CryptoExchange.Net/Objects/Options.cs @@ -10,7 +10,7 @@ using Microsoft.Extensions.Logging; namespace CryptoExchange.Net.Objects { /// - /// Base options + /// Base options, applicable to everything /// public class BaseOptions { @@ -50,7 +50,7 @@ namespace CryptoExchange.Net.Objects } /// - /// Base client options + /// Client options, for both the socket and rest clients /// public class BaseClientOptions : BaseOptions { @@ -60,7 +60,7 @@ namespace CryptoExchange.Net.Objects public ApiProxy? Proxy { get; set; } /// - /// Api credentials to be used for all api clients unless overriden + /// Api credentials to be used for signing requests to private endpoints. These credentials will be used for each API in the client, unless overriden in the API options /// public ApiCredentials? ApiCredentials { get; set; } @@ -86,7 +86,7 @@ namespace CryptoExchange.Net.Objects } /// - /// Base for rest client options + /// Rest client options /// public class BaseRestClientOptions : BaseClientOptions { @@ -96,7 +96,7 @@ namespace CryptoExchange.Net.Objects public TimeSpan RequestTimeout { get; set; } = TimeSpan.FromSeconds(30); /// - /// Http client to use. If a HttpClient is provided in this property the RequestTimeout and Proxy options will be ignored in requests and should be set on the provided HttpClient instance + /// Http client to use. If a HttpClient is provided in this property the RequestTimeout and Proxy options provided in these options will be ignored in requests and should be set on the provided HttpClient instance /// public HttpClient? HttpClient { get; set; } @@ -122,7 +122,7 @@ namespace CryptoExchange.Net.Objects } /// - /// Base for socket client options + /// Socket client options /// public class BaseSocketClientOptions : BaseClientOptions { @@ -196,32 +196,49 @@ namespace CryptoExchange.Net.Objects } } + /// + /// API client options + /// public class ApiClientOptions { /// - /// If true, the CallResult and DataEvent objects will also include the originally received json data in the OriginalData property + /// The base address of the API /// public string BaseAddress { get; set; } /// - /// The api credentials used for signing requests + /// The api credentials used for signing requests to this API. Overrides API credentials provided in the client options /// public ApiCredentials? ApiCredentials { get; set; } - + /// + /// ctor + /// +#pragma warning disable 8618 // Will always get filled by the implementation public ApiClientOptions() { } +#pragma warning restore 8618 - public ApiClientOptions(string baseAddres) + /// + /// ctor + /// + /// Base address for the API + public ApiClientOptions(string baseAddress) { - BaseAddress = baseAddres; + BaseAddress = baseAddress; } + /// + /// ctor + /// + /// Copy values for the provided options +#pragma warning disable 8618 // Will always get filled by the provided options public ApiClientOptions(ApiClientOptions baseOn) { Copy(this, baseOn); } +#pragma warning restore 8618 /// /// Copy the values of the def to the input @@ -243,6 +260,9 @@ namespace CryptoExchange.Net.Objects } } + /// + /// Rest API client options + /// public class RestApiClientOptions: ApiClientOptions { /// @@ -255,14 +275,25 @@ namespace CryptoExchange.Net.Objects /// public RateLimitingBehaviour RateLimitingBehaviour { get; set; } = RateLimitingBehaviour.Wait; + /// + /// ctor + /// public RestApiClientOptions() { } + /// + /// ctor + /// + /// Base address for the API public RestApiClientOptions(string baseAddress): base(baseAddress) { } + /// + /// ctor + /// + /// Copy values for the provided options public RestApiClientOptions(RestApiClientOptions baseOn) { Copy(this, baseOn);