From 2f983498b3e828cedaa957a78cd7b2bf09b761a2 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Sat, 25 Oct 2025 13:10:27 +0200 Subject: [PATCH 1/9] Make RestClientContext a `record` and move logic to RestClientBuilder --- .../RestClientBuilderJsonExtensions.cs | 14 +- .../RestClientTests.cs | 3 +- ...stClientBuilderNewtonsoftJsonExtensions.cs | 5 +- .../Implementation/DummyRequestLogger.cs | 19 ++ .../Implementation/RestClient.cs | 85 ++---- .../Implementation/RestClientBuilder.cs | 247 +++++++++++------- .../Implementation/RestClientContext.cs | 67 ++--- .../Implementation/StringSerializer.cs | 25 +- 8 files changed, 241 insertions(+), 224 deletions(-) create mode 100644 Activout.RestClient/Implementation/DummyRequestLogger.cs diff --git a/Activout.RestClient.Json/RestClientBuilderJsonExtensions.cs b/Activout.RestClient.Json/RestClientBuilderJsonExtensions.cs index 26f67dd..19e905c 100644 --- a/Activout.RestClient.Json/RestClientBuilderJsonExtensions.cs +++ b/Activout.RestClient.Json/RestClientBuilderJsonExtensions.cs @@ -1,4 +1,5 @@ using System.Text.Json; +using static Activout.RestClient.Json.SystemTextJsonDefaults; namespace Activout.RestClient.Json; @@ -12,15 +13,18 @@ public static class RestClientBuilderJsonExtensions /// /// The REST client builder instance. /// Optional custom JSON serializer options. If not provided, default options will be used. - /// Optional list of content types to support. If not provided, defaults will be used. + /// Optional list of content types to support. If not provided, defaults will be used. /// The REST client builder instance. public static IRestClientBuilder WithSystemTextJson(this IRestClientBuilder builder, JsonSerializerOptions? jsonSerializerOptions = null, - MediaType[]? supportedMediaTypes = null) + MediaType[]? mediaTypes = null) { - // Register the serializer and deserializer - builder.With(new SystemTextJsonSerializer(jsonSerializerOptions, supportedMediaTypes)); - builder.With(new SystemTextJsonDeserializer(jsonSerializerOptions, supportedMediaTypes)); + mediaTypes ??= MediaTypes; + + builder.With(new SystemTextJsonSerializer(jsonSerializerOptions, mediaTypes)); + builder.With(new SystemTextJsonDeserializer(jsonSerializerOptions, mediaTypes)); + builder.Accept(string.Join(", ", mediaTypes.Select(type => type.Value))); + builder.ContentType(mediaTypes.First()); return builder; } diff --git a/Activout.RestClient.Newtonsoft.Json.Test/RestClientTests.cs b/Activout.RestClient.Newtonsoft.Json.Test/RestClientTests.cs index ce963eb..4b6bd42 100644 --- a/Activout.RestClient.Newtonsoft.Json.Test/RestClientTests.cs +++ b/Activout.RestClient.Newtonsoft.Json.Test/RestClientTests.cs @@ -4,7 +4,6 @@ using System.Net.Http; using System.Text; using System.Threading.Tasks; -using Activout.RestClient.Helpers.Implementation; using Activout.RestClient.Newtonsoft.Json.Test.MovieReviews; using Microsoft.Extensions.Logging; using Newtonsoft.Json; @@ -91,7 +90,7 @@ public void TestErrorSync() // arrange _mockHttp .When(HttpMethod.Get, $"{BaseUri}/movies/{MovieId}/reviews/{ReviewId}") - .Respond(HttpStatusCode.NotFound, request => new StringContent(JsonConvert.SerializeObject(new + .Respond(HttpStatusCode.NotFound, _ => new StringContent(JsonConvert.SerializeObject(new { Errors = new object[] { diff --git a/Activout.RestClient.Newtonsoft.Json/RestClientBuilderNewtonsoftJsonExtensions.cs b/Activout.RestClient.Newtonsoft.Json/RestClientBuilderNewtonsoftJsonExtensions.cs index 5dd23a7..f744b19 100644 --- a/Activout.RestClient.Newtonsoft.Json/RestClientBuilderNewtonsoftJsonExtensions.cs +++ b/Activout.RestClient.Newtonsoft.Json/RestClientBuilderNewtonsoftJsonExtensions.cs @@ -1,4 +1,5 @@ using Newtonsoft.Json; +using static Activout.RestClient.Newtonsoft.Json.NewtonsoftJsonDefaults; namespace Activout.RestClient.Newtonsoft.Json; @@ -7,10 +8,12 @@ public static class RestClientBuilderNewtonsoftJsonExtensions public static IRestClientBuilder WithNewtonsoftJson(this IRestClientBuilder builder, JsonSerializerSettings? jsonSerializerSettings = null) { - var settings = jsonSerializerSettings ?? NewtonsoftJsonDefaults.DefaultJsonSerializerSettings; + var settings = jsonSerializerSettings ?? DefaultJsonSerializerSettings; builder.With(new NewtonsoftJsonSerializer(settings)); builder.With(new NewtonsoftJsonDeserializer(settings)); + builder.Accept(string.Join(", ", SupportedMediaTypes.Select(type => type.Value))); + builder.ContentType(SupportedMediaTypes.First()); return builder; } diff --git a/Activout.RestClient/Implementation/DummyRequestLogger.cs b/Activout.RestClient/Implementation/DummyRequestLogger.cs new file mode 100644 index 0000000..3719b88 --- /dev/null +++ b/Activout.RestClient/Implementation/DummyRequestLogger.cs @@ -0,0 +1,19 @@ +using System; +using System.Net.Http; + +namespace Activout.RestClient.Implementation; + +internal sealed class DummyRequestLogger : IRequestLogger, IDisposable +{ + public static IRequestLogger Instance { get; } = new DummyRequestLogger(); + + public IDisposable TimeOperation(HttpRequestMessage httpRequestMessage) + { + return this; + } + + public void Dispose() + { + // Do nothing + } +} \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClient.cs b/Activout.RestClient/Implementation/RestClient.cs index 07310a5..fe12ce0 100644 --- a/Activout.RestClient/Implementation/RestClient.cs +++ b/Activout.RestClient/Implementation/RestClient.cs @@ -1,76 +1,47 @@ -#nullable disable using System; using System.Collections.Concurrent; using System.Collections.Generic; using System.Dynamic; using System.Linq; using System.Reflection; -using Activout.RestClient.DomainExceptions; -namespace Activout.RestClient.Implementation +namespace Activout.RestClient.Implementation; + +internal class RestClient : DynamicObject { - internal class RestClient : DynamicObject where T : class - { - private readonly RestClientContext _context; - private readonly Type _type; + private readonly RestClientContext _context; + private readonly Type _type; - private readonly IDictionary _requestHandlers = - new ConcurrentDictionary(); + private readonly IDictionary _requestHandlers = + new ConcurrentDictionary(); - public RestClient(RestClientContext context) - { - _type = typeof(T); - _context = context; - HandleAttributes(); - _context.DefaultSerializer = _context.SerializationManager.GetSerializer(_context.DefaultContentType); - } + public RestClient(Type type, RestClientContext context) + { + _type = type; + _context = context; + } - private void HandleAttributes() - { - var attributes = _type.GetCustomAttributes(); - foreach (var attribute in attributes) - switch (attribute) - { - case ContentTypeAttribute contentTypeAttribute: - _context.DefaultContentType = MediaType.ValueOf(contentTypeAttribute.ContentType); - break; - case DomainExceptionAttribute domainExceptionAttribute: - _context.DomainExceptionType = domainExceptionAttribute.Type; - break; - case ErrorResponseAttribute errorResponseAttribute: - _context.ErrorResponseType = errorResponseAttribute.Type; - break; - case HeaderAttribute headerAttribute: - _context.DefaultHeaders.AddOrReplaceHeader(headerAttribute.Name, headerAttribute.Value, headerAttribute.Replace); - break; - case PathAttribute pathAttribute: - _context.BaseTemplate = pathAttribute.Template; - break; - } - } + public override IEnumerable GetDynamicMemberNames() + { + return _type.GetMembers().Select(x => x.Name); + } - public override IEnumerable GetDynamicMemberNames() + public override bool TryInvokeMember(InvokeMemberBinder binder, object?[]? args, out object? result) + { + var method = _type.GetMethod(binder.Name); + if (method == null) { - return _type.GetMembers().Select(x => x.Name); + result = null; + return false; } - public override bool TryInvokeMember(InvokeMemberBinder binder, object[] args, out object result) + if (!_requestHandlers.TryGetValue(method, out var requestHandler)) { - var method = _type.GetMethod(binder.Name); - if (method == null) - { - result = null; - return false; - } - - if (!_requestHandlers.TryGetValue(method, out var requestHandler)) - { - requestHandler = new RequestHandler(method, _context); - _requestHandlers[method] = requestHandler; - } - - result = requestHandler.Send(args); - return true; + requestHandler = new RequestHandler(method, _context); + _requestHandlers[method] = requestHandler; } + + result = requestHandler.Send(args); + return true; } } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClientBuilder.cs b/Activout.RestClient/Implementation/RestClientBuilder.cs index ca3a3d3..64dcb16 100644 --- a/Activout.RestClient/Implementation/RestClientBuilder.cs +++ b/Activout.RestClient/Implementation/RestClientBuilder.cs @@ -1,8 +1,8 @@ -#nullable disable using System; using System.Collections.Generic; using System.Linq; using System.Net.Http; +using System.Reflection; using Activout.RestClient.DomainExceptions; using Activout.RestClient.Helpers; using Activout.RestClient.Helpers.Implementation; @@ -11,124 +11,175 @@ using Activout.RestClient.Serialization; using Activout.RestClient.Serialization.Implementation; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; -namespace Activout.RestClient.Implementation +namespace Activout.RestClient.Implementation; + +internal class RestClientBuilder : IRestClientBuilder { - internal class RestClientBuilder : IRestClientBuilder + private static readonly MediaType DefaultContentType = new MediaType("text/plain"); + + private IDuckTyping _duckTyping = DuckTyping.Instance; + private readonly List _serializers = SerializationManager.DefaultSerializers.ToList(); + private readonly List _deserializers = SerializationManager.DefaultDeserializers.ToList(); + private ILogger? _logger; + private Uri? _baseUri; + private string _baseTemplate = ""; + private ISerializer? _defaultSerializer; + private ISerializationManager? _serializationManager; + private HttpClient? _httpClient; + private ITaskConverterFactory? _taskConverterFactory; + private Type? _errorResponseType; + private MediaType? _defaultContentType; + private IParamConverterManager? _paramConverterManager = null; + private List> _defaultHeaders = new List>(); + private IRequestLogger? _requestLogger; + private Type? _domainExceptionType; + private IDomainExceptionMapperFactory? _domainExceptionMapperFactory; + + public IRestClientBuilder BaseUri(Uri apiUri) { - private IDuckTyping _duckTyping = DuckTyping.Instance; - - private readonly RestClientContext _context = new RestClientContext() - { - ParamConverterManager = ParamConverterManager.Instance, - TaskConverterFactory = TaskConverter3Factory.Instance - }; - private readonly List _serializers = SerializationManager.DefaultSerializers.ToList(); - private readonly List _deserializers = SerializationManager.DefaultDeserializers.ToList(); - - public IRestClientBuilder BaseUri(Uri apiUri) - { - _context.BaseUri = AddTrailingSlash(apiUri); - return this; - } + _baseUri = AddTrailingSlash(apiUri); + return this; + } - public IRestClientBuilder ContentType(MediaType contentType) - { - _context.DefaultContentType = contentType; - return this; - } + public IRestClientBuilder ContentType(MediaType contentType) + { + _defaultContentType = contentType; + return this; + } - public IRestClientBuilder Header(string name, object value) - { - _context.DefaultHeaders.Add(new KeyValuePair(name, value)); - return this; - } + public IRestClientBuilder Header(string name, object value) + { + _defaultHeaders.Add(new KeyValuePair(name, value)); + return this; + } - public IRestClientBuilder With(IDuckTyping duckTyping) - { - _duckTyping = duckTyping; - return this; - } + public IRestClientBuilder With(IDuckTyping duckTyping) + { + _duckTyping = duckTyping; + return this; + } - public IRestClientBuilder With(ILogger logger) - { - _context.Logger = logger; - return this; - } + public IRestClientBuilder With(ILogger logger) + { + _logger = logger; + return this; + } - public IRestClientBuilder With(HttpClient httpClient) - { - _context.HttpClient = httpClient; - return this; - } + public IRestClientBuilder With(HttpClient httpClient) + { + _httpClient = httpClient; + return this; + } - public IRestClientBuilder With(IRequestLogger requestLogger) - { - _context.RequestLogger = requestLogger; - return this; - } + public IRestClientBuilder With(IRequestLogger requestLogger) + { + _requestLogger = requestLogger; + return this; + } - public IRestClientBuilder With(IDeserializer deserializer) - { - _deserializers.Add(deserializer); - return this; - } + public IRestClientBuilder With(IDeserializer deserializer) + { + _deserializers.Add(deserializer); + return this; + } - public IRestClientBuilder With(ISerializer serializer) - { - _serializers.Add(serializer); - return this; - } + public IRestClientBuilder With(ISerializer serializer) + { + _serializers.Add(serializer); + return this; + } - public IRestClientBuilder With(ISerializationManager serializationManager) - { - _context.SerializationManager = serializationManager; - return this; - } + public IRestClientBuilder With(ISerializationManager serializationManager) + { + _serializationManager = serializationManager; + return this; + } - public IRestClientBuilder With(ITaskConverterFactory taskConverterFactory) - { - _context.TaskConverterFactory = taskConverterFactory; - return this; - } + public IRestClientBuilder With(ITaskConverterFactory taskConverterFactory) + { + _taskConverterFactory = taskConverterFactory; + return this; + } - public IRestClientBuilder With(IDomainExceptionMapperFactory domainExceptionMapperFactory) - { - _context.DomainExceptionMapperFactory = domainExceptionMapperFactory; - return this; - } + public IRestClientBuilder With(IDomainExceptionMapperFactory domainExceptionMapperFactory) + { + _domainExceptionMapperFactory = domainExceptionMapperFactory; + return this; + } - public T Build() where T : class - { - if (_context.HttpClient == null) - { - _context.HttpClient = new HttpClient(); - } + public IRestClientBuilder With(IParamConverterManager paramConverterManager) + { + _paramConverterManager = paramConverterManager; + return this; + } - if (_context.SerializationManager == null) - { - _context.SerializationManager = new SerializationManager(_serializers, _deserializers); - } + public T Build() where T : class + { + var type = typeof(T); + HandleAttributes(type); + + _defaultContentType ??= DefaultContentType; + _serializationManager ??= new SerializationManager(_serializers, _deserializers); + _defaultSerializer ??= _serializationManager.GetSerializer(_defaultContentType) ?? StringSerializer.Instance; + + var context = new RestClientContext( + BaseUri: _baseUri ?? throw new InvalidOperationException("BaseUri is not set."), + BaseTemplate: _baseTemplate, + DefaultSerializer: _defaultSerializer, + SerializationManager: _serializationManager, + HttpClient: _httpClient ?? new HttpClient(), + TaskConverterFactory: _taskConverterFactory ?? TaskConverter3Factory.Instance, + ErrorResponseType: _errorResponseType, + DefaultContentType: _defaultContentType, + ParamConverterManager: _paramConverterManager ?? ParamConverterManager.Instance, + DomainExceptionType: _domainExceptionType, + DomainExceptionMapperFactory: _domainExceptionMapperFactory ?? + new DefaultDomainExceptionMapperFactory(), + DefaultHeaders: _defaultHeaders, + Logger: _logger ?? NullLogger.Instance, + RequestLogger: _requestLogger ?? DummyRequestLogger.Instance + ); + + var client = new RestClient(type, context); + return _duckTyping.DuckType(client); + } - if (_context.DomainExceptionMapperFactory == null) + private void HandleAttributes(Type type) + { + var attributes = type.GetCustomAttributes(); + foreach (var attribute in attributes) + switch (attribute) { - _context.DomainExceptionMapperFactory = new DefaultDomainExceptionMapperFactory(); + case ContentTypeAttribute contentTypeAttribute: + _defaultContentType = MediaType.ValueOf(contentTypeAttribute.ContentType); + break; + case DomainExceptionAttribute domainExceptionAttribute: + _domainExceptionType = domainExceptionAttribute.Type; + break; + case ErrorResponseAttribute errorResponseAttribute: + _errorResponseType = errorResponseAttribute.Type; + break; + case HeaderAttribute headerAttribute: + _defaultHeaders.AddOrReplaceHeader(headerAttribute.Name, headerAttribute.Value, + headerAttribute.Replace); + break; + case PathAttribute pathAttribute: + _baseTemplate = pathAttribute.Template!; + break; } + } - var client = new RestClient(_context); - return _duckTyping.DuckType(client); - } - - private static Uri AddTrailingSlash(Uri apiUri) + private static Uri AddTrailingSlash(Uri apiUri) + { + var uriBuilder = new UriBuilder(apiUri ?? throw new ArgumentNullException(nameof(apiUri))); + if (uriBuilder.Path.EndsWith("/")) { - var uriBuilder = new UriBuilder(apiUri ?? throw new ArgumentNullException(nameof(apiUri))); - if (uriBuilder.Path.EndsWith("/")) - { - return apiUri; - } - - uriBuilder.Path = uriBuilder.Path + "/"; - return uriBuilder.Uri; + return apiUri; } + + uriBuilder.Path = uriBuilder.Path + "/"; + return uriBuilder.Uri; } } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClientContext.cs b/Activout.RestClient/Implementation/RestClientContext.cs index f41d9ba..823ea80 100644 --- a/Activout.RestClient/Implementation/RestClientContext.cs +++ b/Activout.RestClient/Implementation/RestClientContext.cs @@ -1,60 +1,29 @@ -#nullable disable using System; using System.Collections.Generic; -using System.Collections.ObjectModel; -using System.Linq; using System.Net.Http; using Activout.RestClient.DomainExceptions; using Activout.RestClient.Helpers; using Activout.RestClient.ParamConverter; using Activout.RestClient.Serialization; using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -namespace Activout.RestClient.Implementation -{ - internal class RestClientContext - { - private static readonly Collection JsonMediaTypeCollection = new Collection - { - new MediaType("application/json") - }; - - public RestClientContext() - { - BaseTemplate = ""; - DefaultContentType = JsonMediaTypeCollection.FirstOrDefault(); - DefaultHeaders = new List>(); - ErrorResponseType = typeof(string); - } - - public ILogger Logger { get; internal set; } = NullLogger.Instance; - public Uri BaseUri { get; internal set; } - public string BaseTemplate { get; internal set; } - public ISerializer DefaultSerializer { get; internal set; } - public ISerializationManager SerializationManager { get; internal set; } - public HttpClient HttpClient { get; internal set; } - public ITaskConverterFactory TaskConverterFactory { get; internal set; } - public Type ErrorResponseType { get; internal set; } - public MediaType DefaultContentType { get; internal set; } - public IParamConverterManager ParamConverterManager { get; internal set; } - public List> DefaultHeaders { get; } - public IRequestLogger RequestLogger { get; set; } = new DummyRequestLogger(); - public Type DomainExceptionType { get; set; } - public IDomainExceptionMapperFactory DomainExceptionMapperFactory { get; set; } - public bool UseDomainException => DomainExceptionType != null; - } +namespace Activout.RestClient.Implementation; - internal sealed class DummyRequestLogger : IRequestLogger, IDisposable - { - public IDisposable TimeOperation(HttpRequestMessage httpRequestMessage) - { - return this; - } - - public void Dispose() - { - // Do nothing - } - } +internal record RestClientContext( + Uri BaseUri, + string BaseTemplate, + ISerializer DefaultSerializer, + ISerializationManager SerializationManager, + HttpClient HttpClient, + ITaskConverterFactory TaskConverterFactory, + Type? ErrorResponseType, + MediaType? DefaultContentType, + IParamConverterManager ParamConverterManager, + Type? DomainExceptionType, + IDomainExceptionMapperFactory DomainExceptionMapperFactory, + List> DefaultHeaders, + ILogger Logger, + IRequestLogger RequestLogger) +{ + public bool UseDomainException { get; } = DomainExceptionType != null; } \ No newline at end of file diff --git a/Activout.RestClient/Serialization/Implementation/StringSerializer.cs b/Activout.RestClient/Serialization/Implementation/StringSerializer.cs index 43514da..80c06ee 100644 --- a/Activout.RestClient/Serialization/Implementation/StringSerializer.cs +++ b/Activout.RestClient/Serialization/Implementation/StringSerializer.cs @@ -1,20 +1,21 @@ using System.Net.Http; using System.Text; -namespace Activout.RestClient.Serialization.Implementation +namespace Activout.RestClient.Serialization.Implementation; + +public class StringSerializer : ISerializer { - internal class StringSerializer : ISerializer - { - public int Order { get; set; } = 1000; + public static ISerializer Instance { get; } = new StringSerializer(); + + public int Order { get; set; } = 1000; - public HttpContent Serialize(object? data, Encoding encoding, MediaType mediaType) - { - return new StringContent(data?.ToString() ?? "", encoding, mediaType.Value); - } + public HttpContent Serialize(object? data, Encoding encoding, MediaType mediaType) + { + return new StringContent(data?.ToString() ?? "", encoding, mediaType.Value); + } - public bool CanSerialize(MediaType mediaType) - { - return mediaType.Value.StartsWith("text/"); - } + public bool CanSerialize(MediaType mediaType) + { + return mediaType.Value.StartsWith("text/"); } } \ No newline at end of file From 739544192adb694dd9741f5d7bdf70a19debc97e Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Sat, 25 Oct 2025 13:39:37 +0200 Subject: [PATCH 2/9] Coderabbit nitpicking --- Activout.RestClient/IRestClientBuilder.cs | 31 +++++++++---------- .../Implementation/DummyRequestLogger.cs | 5 +-- .../Implementation/HeaderListExtensions.cs | 21 ++++++------- .../Implementation/RestClientBuilder.cs | 14 ++++----- .../Implementation/RestClientContext.cs | 4 +-- .../Implementation/StringSerializer.cs | 15 ++++----- 6 files changed, 41 insertions(+), 49 deletions(-) diff --git a/Activout.RestClient/IRestClientBuilder.cs b/Activout.RestClient/IRestClientBuilder.cs index 1c727e2..9a0f3db 100644 --- a/Activout.RestClient/IRestClientBuilder.cs +++ b/Activout.RestClient/IRestClientBuilder.cs @@ -6,21 +6,20 @@ using Activout.RestClient.Serialization; using Microsoft.Extensions.Logging; -namespace Activout.RestClient +namespace Activout.RestClient; + +public interface IRestClientBuilder { - public interface IRestClientBuilder - { - IRestClientBuilder BaseUri(Uri apiUri); - IRestClientBuilder ContentType(MediaType contentType); - IRestClientBuilder Header(string name, object value); - IRestClientBuilder With(ILogger logger); - IRestClientBuilder With(HttpClient httpClient); - IRestClientBuilder With(IRequestLogger requestLogger); - IRestClientBuilder With(IDeserializer deserializer); - IRestClientBuilder With(ISerializer serializer); - IRestClientBuilder With(ISerializationManager serializationManager); - IRestClientBuilder With(ITaskConverterFactory taskConverterFactory); - IRestClientBuilder With(IDomainExceptionMapperFactory domainExceptionMapperFactory); - T Build() where T : class; - } + IRestClientBuilder BaseUri(Uri apiUri); + IRestClientBuilder ContentType(MediaType contentType); + IRestClientBuilder Header(string name, object value, bool isReplace = false); + IRestClientBuilder With(ILogger logger); + IRestClientBuilder With(HttpClient httpClient); + IRestClientBuilder With(IRequestLogger requestLogger); + IRestClientBuilder With(IDeserializer deserializer); + IRestClientBuilder With(ISerializer serializer); + IRestClientBuilder With(ISerializationManager serializationManager); + IRestClientBuilder With(ITaskConverterFactory taskConverterFactory); + IRestClientBuilder With(IDomainExceptionMapperFactory domainExceptionMapperFactory); + T Build() where T : class; } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/DummyRequestLogger.cs b/Activout.RestClient/Implementation/DummyRequestLogger.cs index 3719b88..3635e46 100644 --- a/Activout.RestClient/Implementation/DummyRequestLogger.cs +++ b/Activout.RestClient/Implementation/DummyRequestLogger.cs @@ -7,10 +7,7 @@ internal sealed class DummyRequestLogger : IRequestLogger, IDisposable { public static IRequestLogger Instance { get; } = new DummyRequestLogger(); - public IDisposable TimeOperation(HttpRequestMessage httpRequestMessage) - { - return this; - } + public IDisposable TimeOperation(HttpRequestMessage httpRequestMessage) => this; public void Dispose() { diff --git a/Activout.RestClient/Implementation/HeaderListExtensions.cs b/Activout.RestClient/Implementation/HeaderListExtensions.cs index f5f51a2..be0cacc 100644 --- a/Activout.RestClient/Implementation/HeaderListExtensions.cs +++ b/Activout.RestClient/Implementation/HeaderListExtensions.cs @@ -1,20 +1,19 @@ using System; using System.Collections.Generic; -namespace Activout.RestClient.Implementation +namespace Activout.RestClient.Implementation; + +public static class HeaderListExtensions { - public static class HeaderListExtensions + public static void AddOrReplaceHeader(this List> headers, string name, + T value, bool replace) { - public static void AddOrReplaceHeader(this List> headers, string name, - string value, bool replace) + if (replace) { - if (replace) - { - headers.RemoveAll(header => - header.Key.Equals(name, StringComparison.InvariantCultureIgnoreCase)); - } - - headers.Add(new KeyValuePair(name, value)); + headers.RemoveAll(header => + header.Key.Equals(name, StringComparison.OrdinalIgnoreCase)); } + + headers.Add(new KeyValuePair(name, value)); } } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClientBuilder.cs b/Activout.RestClient/Implementation/RestClientBuilder.cs index 64dcb16..f48f83b 100644 --- a/Activout.RestClient/Implementation/RestClientBuilder.cs +++ b/Activout.RestClient/Implementation/RestClientBuilder.cs @@ -31,8 +31,8 @@ internal class RestClientBuilder : IRestClientBuilder private ITaskConverterFactory? _taskConverterFactory; private Type? _errorResponseType; private MediaType? _defaultContentType; - private IParamConverterManager? _paramConverterManager = null; - private List> _defaultHeaders = new List>(); + private IParamConverterManager? _paramConverterManager; + private readonly List> _defaultHeaders = new List>(); private IRequestLogger? _requestLogger; private Type? _domainExceptionType; private IDomainExceptionMapperFactory? _domainExceptionMapperFactory; @@ -49,9 +49,9 @@ public IRestClientBuilder ContentType(MediaType contentType) return this; } - public IRestClientBuilder Header(string name, object value) + public IRestClientBuilder Header(string name, object value, bool isReplace = false) { - _defaultHeaders.Add(new KeyValuePair(name, value)); + _defaultHeaders.AddOrReplaceHeader(name, value, isReplace); return this; } @@ -137,7 +137,7 @@ public T Build() where T : class DomainExceptionType: _domainExceptionType, DomainExceptionMapperFactory: _domainExceptionMapperFactory ?? new DefaultDomainExceptionMapperFactory(), - DefaultHeaders: _defaultHeaders, + DefaultHeaders: new List>(_defaultHeaders), Logger: _logger ?? NullLogger.Instance, RequestLogger: _requestLogger ?? DummyRequestLogger.Instance ); @@ -174,12 +174,12 @@ private void HandleAttributes(Type type) private static Uri AddTrailingSlash(Uri apiUri) { var uriBuilder = new UriBuilder(apiUri ?? throw new ArgumentNullException(nameof(apiUri))); - if (uriBuilder.Path.EndsWith("/")) + if (uriBuilder.Path.EndsWith('/')) { return apiUri; } - uriBuilder.Path = uriBuilder.Path + "/"; + uriBuilder.Path += "/"; return uriBuilder.Uri; } } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClientContext.cs b/Activout.RestClient/Implementation/RestClientContext.cs index 823ea80..41d6fb0 100644 --- a/Activout.RestClient/Implementation/RestClientContext.cs +++ b/Activout.RestClient/Implementation/RestClientContext.cs @@ -21,9 +21,9 @@ internal record RestClientContext( IParamConverterManager ParamConverterManager, Type? DomainExceptionType, IDomainExceptionMapperFactory DomainExceptionMapperFactory, - List> DefaultHeaders, + IReadOnlyList> DefaultHeaders, ILogger Logger, IRequestLogger RequestLogger) { - public bool UseDomainException { get; } = DomainExceptionType != null; + public bool UseDomainException => DomainExceptionType != null; } \ No newline at end of file diff --git a/Activout.RestClient/Serialization/Implementation/StringSerializer.cs b/Activout.RestClient/Serialization/Implementation/StringSerializer.cs index 80c06ee..1147123 100644 --- a/Activout.RestClient/Serialization/Implementation/StringSerializer.cs +++ b/Activout.RestClient/Serialization/Implementation/StringSerializer.cs @@ -1,21 +1,18 @@ +using System; using System.Net.Http; using System.Text; namespace Activout.RestClient.Serialization.Implementation; -public class StringSerializer : ISerializer +public sealed class StringSerializer : ISerializer { public static ISerializer Instance { get; } = new StringSerializer(); public int Order { get; set; } = 1000; - public HttpContent Serialize(object? data, Encoding encoding, MediaType mediaType) - { - return new StringContent(data?.ToString() ?? "", encoding, mediaType.Value); - } + public HttpContent Serialize(object? data, Encoding encoding, MediaType mediaType) => + new StringContent(data?.ToString() ?? "", encoding, mediaType.Value); - public bool CanSerialize(MediaType mediaType) - { - return mediaType.Value.StartsWith("text/"); - } + public bool CanSerialize(MediaType mediaType) => + mediaType.Value.StartsWith("text/", StringComparison.OrdinalIgnoreCase); } \ No newline at end of file From 055ad6a91fc5d3f6fde19053e4f21bde969a08f7 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Sat, 25 Oct 2025 15:49:39 +0200 Subject: [PATCH 3/9] Fix more Coderabbit feedback --- .../NewtonsoftJsonDeserializerTest.cs | 30 ++++++--------- Activout.RestClient/IRestClientBuilder.cs | 6 ++- .../Implementation/DummyRequestLogger.cs | 5 ++- .../Implementation/RestClientBuilder.cs | 36 +++++++++++++++--- .../RestClientBuilderExtensions.cs | 37 +++++++++---------- .../Serialization/IDeserializer.cs | 13 +++---- .../Serialization/ISerializer.cs | 13 +++---- .../Implementation/SerializationManager.cs | 5 +-- .../Implementation/StringDeserializer.cs | 29 +++++++-------- .../Implementation/StringSerializer.cs | 2 +- 10 files changed, 97 insertions(+), 79 deletions(-) diff --git a/Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs b/Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs index 03546b7..68f877b 100644 --- a/Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs +++ b/Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs @@ -41,9 +41,9 @@ public void TestDefaultSettingsPascalCase() // Arrange _mockHttp.Expect(BaseUri) .Respond(new StringContent(JsonConvert.SerializeObject(new - { - Value = "PascalCase" - }), + { + Value = "PascalCase" + }), Encoding.UTF8, "application/json")); @@ -62,9 +62,9 @@ private void MockCamelCaseResponse() { _mockHttp.Expect(BaseUri) .Respond(new StringContent(JsonConvert.SerializeObject(new - { - value = "CamelCase" - }), + { + value = "CamelCase" + }), Encoding.UTF8, "application/json")); } @@ -91,20 +91,12 @@ public void TestCamelCaseWithDeserializer() } [Fact] - public void TestCamelCaseWithSerializationManager() + public void TestCamelCaseSerializerSettings() { // Arrange MockCamelCaseResponse(); - var deserializers = SerializationManager.DefaultDeserializers.ToList(); - deserializers.Add(new NewtonsoftJsonDeserializer(new JsonSerializerSettings - { - ContractResolver = new CamelCasePropertyNamesContractResolver() - })); - var serializationManager = new SerializationManager(deserializers: deserializers); - - var client = CreateRestClientBuilder() - .With(serializationManager) + var client = CreateRestClientBuilder(NewtonsoftJsonDefaults.CamelCaseSerializerSettings) .Build(); // Act @@ -115,12 +107,12 @@ public void TestCamelCaseWithSerializationManager() Assert.Equal("CamelCase", data.Value); } - private IRestClientBuilder CreateRestClientBuilder() + private IRestClientBuilder CreateRestClientBuilder(JsonSerializerSettings? jsonSerializerSettings = null) { return _restClientFactory.CreateBuilder() - .WithNewtonsoftJson() + .WithNewtonsoftJson(jsonSerializerSettings) .With(_mockHttp.ToHttpClient()) .BaseUri(new Uri(BaseUri)); } } -} +} \ No newline at end of file diff --git a/Activout.RestClient/IRestClientBuilder.cs b/Activout.RestClient/IRestClientBuilder.cs index 9a0f3db..496f3d4 100644 --- a/Activout.RestClient/IRestClientBuilder.cs +++ b/Activout.RestClient/IRestClientBuilder.cs @@ -1,8 +1,8 @@ -#nullable disable using System; using System.Net.Http; using Activout.RestClient.DomainExceptions; using Activout.RestClient.Helpers; +using Activout.RestClient.ParamConverter; using Activout.RestClient.Serialization; using Microsoft.Extensions.Logging; @@ -12,7 +12,8 @@ public interface IRestClientBuilder { IRestClientBuilder BaseUri(Uri apiUri); IRestClientBuilder ContentType(MediaType contentType); - IRestClientBuilder Header(string name, object value, bool isReplace = false); + IRestClientBuilder Header(string name, object value, bool isReplace = true); + IRestClientBuilder With(IDuckTyping duckTyping); IRestClientBuilder With(ILogger logger); IRestClientBuilder With(HttpClient httpClient); IRestClientBuilder With(IRequestLogger requestLogger); @@ -21,5 +22,6 @@ public interface IRestClientBuilder IRestClientBuilder With(ISerializationManager serializationManager); IRestClientBuilder With(ITaskConverterFactory taskConverterFactory); IRestClientBuilder With(IDomainExceptionMapperFactory domainExceptionMapperFactory); + IRestClientBuilder With(IParamConverterManager paramConverterManager); T Build() where T : class; } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/DummyRequestLogger.cs b/Activout.RestClient/Implementation/DummyRequestLogger.cs index 3635e46..a7b3ac0 100644 --- a/Activout.RestClient/Implementation/DummyRequestLogger.cs +++ b/Activout.RestClient/Implementation/DummyRequestLogger.cs @@ -7,10 +7,13 @@ internal sealed class DummyRequestLogger : IRequestLogger, IDisposable { public static IRequestLogger Instance { get; } = new DummyRequestLogger(); + private DummyRequestLogger() + { + } + public IDisposable TimeOperation(HttpRequestMessage httpRequestMessage) => this; public void Dispose() { - // Do nothing } } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClientBuilder.cs b/Activout.RestClient/Implementation/RestClientBuilder.cs index f48f83b..ab953e8 100644 --- a/Activout.RestClient/Implementation/RestClientBuilder.cs +++ b/Activout.RestClient/Implementation/RestClientBuilder.cs @@ -20,8 +20,13 @@ internal class RestClientBuilder : IRestClientBuilder private static readonly MediaType DefaultContentType = new MediaType("text/plain"); private IDuckTyping _duckTyping = DuckTyping.Instance; - private readonly List _serializers = SerializationManager.DefaultSerializers.ToList(); - private readonly List _deserializers = SerializationManager.DefaultDeserializers.ToList(); + + private readonly List + _serializers = new List(); // SerializationManager.DefaultSerializers.ToList(); + + private readonly List + _deserializers = new List(); // SerializationManager.DefaultDeserializers.ToList(); + private ILogger? _logger; private Uri? _baseUri; private string _baseTemplate = ""; @@ -49,7 +54,7 @@ public IRestClientBuilder ContentType(MediaType contentType) return this; } - public IRestClientBuilder Header(string name, object value, bool isReplace = false) + public IRestClientBuilder Header(string name, object value, bool isReplace = true) { _defaultHeaders.AddOrReplaceHeader(name, value, isReplace); return this; @@ -81,18 +86,34 @@ public IRestClientBuilder With(IRequestLogger requestLogger) public IRestClientBuilder With(IDeserializer deserializer) { + if (_serializationManager != null) + { + throw new InvalidOperationException( + "Cannot add custom deserializers when a custom SerializationManager has been set."); + } _deserializers.Add(deserializer); return this; } public IRestClientBuilder With(ISerializer serializer) { + if (_serializationManager != null) + { + throw new InvalidOperationException( + "Cannot add custom serializers when a custom SerializationManager has been set."); + } _serializers.Add(serializer); return this; } public IRestClientBuilder With(ISerializationManager serializationManager) { + if (_serializers.Count > 0 || _deserializers.Count > 0) + { + throw new InvalidOperationException( + "Cannot set a custom SerializationManager when custom serializers or deserializers have been added."); + } + _serializationManager = serializationManager; return this; } @@ -121,7 +142,9 @@ public T Build() where T : class HandleAttributes(type); _defaultContentType ??= DefaultContentType; - _serializationManager ??= new SerializationManager(_serializers, _deserializers); + _serializationManager ??= new SerializationManager( + _serializers.Concat(SerializationManager.DefaultSerializers).ToList(), + _deserializers.Concat(SerializationManager.DefaultDeserializers).ToList()); _defaultSerializer ??= _serializationManager.GetSerializer(_defaultContentType) ?? StringSerializer.Instance; var context = new RestClientContext( @@ -166,7 +189,10 @@ private void HandleAttributes(Type type) headerAttribute.Replace); break; case PathAttribute pathAttribute: - _baseTemplate = pathAttribute.Template!; + if (pathAttribute.Template != null) + { + _baseTemplate = pathAttribute.Template; + } break; } } diff --git a/Activout.RestClient/RestClientBuilderExtensions.cs b/Activout.RestClient/RestClientBuilderExtensions.cs index c4b19b6..8de449c 100644 --- a/Activout.RestClient/RestClientBuilderExtensions.cs +++ b/Activout.RestClient/RestClientBuilderExtensions.cs @@ -1,29 +1,28 @@ using System; using System.Net.Http.Headers; -namespace Activout.RestClient +namespace Activout.RestClient; + +public static class RestClientBuilderExtensions { - public static class RestClientBuilderExtensions + public static IRestClientBuilder BaseUri(this IRestClientBuilder self, string apiUri) { - public static IRestClientBuilder BaseUri(this IRestClientBuilder self, string apiUri) - { - return self.BaseUri(new Uri(apiUri)); - } + return self.BaseUri(new Uri(apiUri)); + } - public static IRestClientBuilder ContentType(this IRestClientBuilder self, string contentType) - { - return self.ContentType(MediaType.ValueOf(contentType)); - } + public static IRestClientBuilder ContentType(this IRestClientBuilder self, string contentType) + { + return self.ContentType(new MediaType(contentType)); + } - public static IRestClientBuilder Accept(this IRestClientBuilder self, string accept) - { - return self.Header("Accept", accept); - } + public static IRestClientBuilder Accept(this IRestClientBuilder self, string accept) + { + return self.Header("Accept", accept); + } - public static IRestClientBuilder Header(this IRestClientBuilder self, - AuthenticationHeaderValue authenticationHeaderValue) - { - return self.Header("Authorization", authenticationHeaderValue); - } + public static IRestClientBuilder Header(this IRestClientBuilder self, + AuthenticationHeaderValue authenticationHeaderValue) + { + return self.Header("Authorization", authenticationHeaderValue); } } \ No newline at end of file diff --git a/Activout.RestClient/Serialization/IDeserializer.cs b/Activout.RestClient/Serialization/IDeserializer.cs index 2a7faec..fb6a58a 100644 --- a/Activout.RestClient/Serialization/IDeserializer.cs +++ b/Activout.RestClient/Serialization/IDeserializer.cs @@ -2,12 +2,11 @@ using System.Net.Http; using System.Threading.Tasks; -namespace Activout.RestClient.Serialization +namespace Activout.RestClient.Serialization; + +public interface IDeserializer { - public interface IDeserializer - { - int Order { get; set; } - Task Deserialize(HttpContent content, Type type); - bool CanDeserialize(MediaType mediaType); - } + int Order { get; } + Task Deserialize(HttpContent content, Type type); + bool CanDeserialize(MediaType mediaType); } \ No newline at end of file diff --git a/Activout.RestClient/Serialization/ISerializer.cs b/Activout.RestClient/Serialization/ISerializer.cs index fb3b8e9..24ca22a 100644 --- a/Activout.RestClient/Serialization/ISerializer.cs +++ b/Activout.RestClient/Serialization/ISerializer.cs @@ -1,12 +1,11 @@ using System.Net.Http; using System.Text; -namespace Activout.RestClient.Serialization +namespace Activout.RestClient.Serialization; + +public interface ISerializer { - public interface ISerializer - { - int Order { get; set; } - HttpContent Serialize(object? data, Encoding encoding, MediaType mediaType); - bool CanSerialize(MediaType mediaType); - } + int Order { get; } + HttpContent Serialize(object? data, Encoding encoding, MediaType mediaType); + bool CanSerialize(MediaType mediaType); } \ No newline at end of file diff --git a/Activout.RestClient/Serialization/Implementation/SerializationManager.cs b/Activout.RestClient/Serialization/Implementation/SerializationManager.cs index 351171a..ee2cf86 100644 --- a/Activout.RestClient/Serialization/Implementation/SerializationManager.cs +++ b/Activout.RestClient/Serialization/Implementation/SerializationManager.cs @@ -10,7 +10,7 @@ public class SerializationManager : ISerializationManager public static readonly IReadOnlyCollection DefaultSerializers = new List { new FormUrlEncodedSerializer(), - new StringSerializer(), + StringSerializer.Instance, new ByteArraySerializer() } .ToImmutableList(); @@ -46,5 +46,4 @@ public SerializationManager(IReadOnlyCollection? serializers = null return Serializers.FirstOrDefault(serializer => serializer.CanSerialize(mediaType)); } } -} - +} \ No newline at end of file diff --git a/Activout.RestClient/Serialization/Implementation/StringDeserializer.cs b/Activout.RestClient/Serialization/Implementation/StringDeserializer.cs index 7d7c7e1..926fa24 100644 --- a/Activout.RestClient/Serialization/Implementation/StringDeserializer.cs +++ b/Activout.RestClient/Serialization/Implementation/StringDeserializer.cs @@ -2,23 +2,22 @@ using System.Net.Http; using System.Threading.Tasks; -namespace Activout.RestClient.Serialization.Implementation +namespace Activout.RestClient.Serialization.Implementation; + +internal class StringDeserializer : IDeserializer { - internal class StringDeserializer : IDeserializer - { - public int Order { get; set; } = 1000; + public int Order { get; init; } = 1000; - public async Task Deserialize(HttpContent content, Type type) - { - var stringData = await content.ReadAsStringAsync(); - return type == typeof(string) - ? stringData - : Activator.CreateInstance(type, stringData); - } + public async Task Deserialize(HttpContent content, Type type) + { + var stringData = await content.ReadAsStringAsync(); + return type == typeof(string) + ? stringData + : Activator.CreateInstance(type, stringData); + } - public bool CanDeserialize(MediaType mediaType) - { - return mediaType.Value.StartsWith("text/"); - } + public bool CanDeserialize(MediaType mediaType) + { + return mediaType.Value.StartsWith("text/"); } } \ No newline at end of file diff --git a/Activout.RestClient/Serialization/Implementation/StringSerializer.cs b/Activout.RestClient/Serialization/Implementation/StringSerializer.cs index 1147123..23726f0 100644 --- a/Activout.RestClient/Serialization/Implementation/StringSerializer.cs +++ b/Activout.RestClient/Serialization/Implementation/StringSerializer.cs @@ -8,7 +8,7 @@ public sealed class StringSerializer : ISerializer { public static ISerializer Instance { get; } = new StringSerializer(); - public int Order { get; set; } = 1000; + public int Order { get; init; } = 1000; public HttpContent Serialize(object? data, Encoding encoding, MediaType mediaType) => new StringContent(data?.ToString() ?? "", encoding, mediaType.Value); From 9e8c47395cac2aae14f800eaba7da4c3ec294d50 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Mon, 27 Oct 2025 15:47:01 +0100 Subject: [PATCH 4/9] Nullable RequestHandler Also improve exception handling (might break edge cases) --- .../DefaultDomainExceptionErrorObjectTests.cs | 21 +- .../DomainExceptionErrorObjectTests.cs | 4 +- .../Implementation/RequestHandler.cs | 847 +++++++++--------- .../Implementation/RestClientContext.cs | 2 +- 4 files changed, 435 insertions(+), 439 deletions(-) diff --git a/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.cs b/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.cs index 17a3263..86c0382 100644 --- a/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.cs +++ b/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.cs @@ -66,9 +66,9 @@ public async Task TestMapApiErrorObject() } [Theory] - [InlineData(HttpStatusCode.BadRequest, MyDomainErrorEnum.ClientError)] - [InlineData(HttpStatusCode.BadGateway, MyDomainErrorEnum.ServerError)] - public async Task TestDeserializerException(HttpStatusCode httpStatusCode, MyDomainErrorEnum error) + [InlineData(HttpStatusCode.BadRequest)] + [InlineData(HttpStatusCode.BadGateway)] + public async Task TestDeserializerException(HttpStatusCode httpStatusCode) { // Arrange _mockHttp @@ -76,18 +76,18 @@ public async Task TestDeserializerException(HttpStatusCode httpStatusCode, MyDom .Respond(_ => HtmlHttpResponseMessage(httpStatusCode)); // Act - var exception = await Assert.ThrowsAsync(() => + var exception = await Assert.ThrowsAsync(() => _defaultMapperApiClient.Api()); // Assert - Assert.Equal(error, exception.Error); + Assert.Equal(httpStatusCode, exception.StatusCode); Assert.IsType(exception.InnerException); } [Theory] - [InlineData(HttpStatusCode.BadRequest, MyDomainErrorEnum.ClientError)] - [InlineData(HttpStatusCode.BadGateway, MyDomainErrorEnum.ServerError)] - public async Task TestNoDeserializerFound(HttpStatusCode httpStatusCode, MyDomainErrorEnum error) + [InlineData(HttpStatusCode.BadRequest)] + [InlineData(HttpStatusCode.BadGateway)] + public async Task TestNoDeserializerFound(HttpStatusCode httpStatusCode) { // Arrange _mockHttp @@ -95,12 +95,11 @@ public async Task TestNoDeserializerFound(HttpStatusCode httpStatusCode, MyDomai .Respond(_ => FoobarHttpResponseMessage(httpStatusCode)); // Act - var exception = await Assert.ThrowsAsync(() => + var exception = await Assert.ThrowsAsync(() => _defaultMapperApiClient.Api()); // Assert - Assert.Equal(error, exception.Error); - Assert.IsType(exception.InnerException); + Assert.Equal(httpStatusCode, exception.StatusCode); } private static HttpResponseMessage JsonHttpResponseMessage(HttpStatusCode httpStatusCode, MyApiError myApiError) diff --git a/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs b/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs index cdf6c66..c30cd36 100644 --- a/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs +++ b/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs @@ -110,11 +110,11 @@ public async Task TestNoDeserializerFound() .Respond(_ => HtmlHttpResponseMessage(HttpStatusCode.BadRequest)); // Act - var exception = await Assert.ThrowsAsync(() => + var exception = await Assert.ThrowsAsync(() => _myApiClient.Api()); // Assert - Assert.Equal(MyDomainErrorEnum.Unknown, exception.Error.ErrorEnum); + Assert.Equal(HttpStatusCode.BadRequest, exception.StatusCode); Assert.IsType(exception.InnerException); } diff --git a/Activout.RestClient/Implementation/RequestHandler.cs b/Activout.RestClient/Implementation/RequestHandler.cs index d50d806..1c910ad 100644 --- a/Activout.RestClient/Implementation/RequestHandler.cs +++ b/Activout.RestClient/Implementation/RequestHandler.cs @@ -1,4 +1,3 @@ -#nullable disable using System; using System.Collections; using System.Collections.Generic; @@ -15,562 +14,560 @@ using Activout.RestClient.Serialization; using Microsoft.Extensions.Logging; -namespace Activout.RestClient.Implementation +namespace Activout.RestClient.Implementation; + +internal class RequestHandler { - internal class RequestHandler + // https://www.w3.org/Protocols/rfc2616/rfc2616-sec7.html#sec7.2.1 + private const string DefaultHttpContentType = "application/octet-stream"; + + // https://tools.ietf.org/html/rfc7578#section-4.4 + private static readonly MediaType DefaultPartContentType = new MediaType("text/plain"); + + private readonly Type _actualReturnType; + private readonly int _bodyArgumentIndex = -1; + private readonly MediaType _contentType; + private readonly RestClientContext _context; + private readonly ITaskConverter? _converter; + private readonly Type _errorResponseType; + private readonly HttpMethod _httpMethod = HttpMethod.Get; + private readonly ParameterInfo[] _parameters; + private readonly Type _returnType; + private readonly ISerializer _serializer; + private readonly string _template; + private readonly IParamConverter[] _paramConverters; + private readonly IDomainExceptionMapper? _domainExceptionMapper; + private readonly List> _requestHeaders = new List>(); + + private bool DebugLoggingEnabled => _context.Logger.IsEnabled(LogLevel.Debug); + + public RequestHandler(MethodInfo method, RestClientContext context) { - // https://www.w3.org/Protocols/rfc2616/rfc2616-sec7.html#sec7.2.1 - private const string DefaultHttpContentType = "application/octet-stream"; - - // https://tools.ietf.org/html/rfc7578#section-4.4 - private static readonly MediaType DefaultPartContentType = new MediaType("text/plain"); - - private readonly Type _actualReturnType; - private readonly int _bodyArgumentIndex = -1; - private readonly MediaType _contentType; - private readonly RestClientContext _context; - private readonly ITaskConverter _converter; - private readonly Type _errorResponseType; - private readonly HttpMethod _httpMethod = HttpMethod.Get; - private readonly ParameterInfo[] _parameters; - private readonly Type _returnType; - private readonly ISerializer _serializer; - private readonly string _template; - private readonly IParamConverter[] _paramConverters; - private readonly IDomainExceptionMapper _domainExceptionMapper; - private readonly List> _requestHeaders = new List>(); - - private bool DebugLoggingEnabled => _context.Logger.IsEnabled(LogLevel.Debug); - - public RequestHandler(MethodInfo method, RestClientContext context) - { - _returnType = method.ReturnType; - _actualReturnType = GetActualReturnType(); - _parameters = method.GetParameters(); - _paramConverters = GetParamConverters(context.ParamConverterManager); - _converter = CreateConverter(context); - _template = context.BaseTemplate ?? ""; - _serializer = context.DefaultSerializer; - _contentType = context.DefaultContentType; - _errorResponseType = context.ErrorResponseType; - _requestHeaders.AddRange(context.DefaultHeaders); - - var templateBuilder = new StringBuilder(context.BaseTemplate ?? ""); - foreach (var attribute in method.GetCustomAttributes(true)) - switch (attribute) - { - case ContentTypeAttribute contentTypeAttribute: - _contentType = MediaType.ValueOf(contentTypeAttribute.ContentType); - break; - - case ErrorResponseAttribute errorResponseAttribute: - _errorResponseType = errorResponseAttribute.Type; - break; - - case HeaderAttribute headerAttribute: - _requestHeaders.AddOrReplaceHeader(headerAttribute.Name, headerAttribute.Value, - headerAttribute.Replace); - break; - - case HttpMethodAttribute httpMethodAttribute: - templateBuilder.Append(httpMethodAttribute.Template); - _httpMethod = GetHttpMethod(httpMethodAttribute); - break; - - case PathAttribute pathAttribute: - templateBuilder.Append(pathAttribute.Template); - break; - } - - if (IsHttpMethodWithBody()) + _returnType = method.ReturnType; + _actualReturnType = GetActualReturnType(); + _parameters = method.GetParameters(); + _paramConverters = GetParamConverters(context.ParamConverterManager); + _converter = CreateConverter(context); + _template = context.BaseTemplate; + _serializer = context.DefaultSerializer; + _contentType = context.DefaultContentType; + _errorResponseType = context.ErrorResponseType ?? typeof(string); + _requestHeaders.AddRange(context.DefaultHeaders); + + var templateBuilder = new StringBuilder(context.BaseTemplate); + foreach (var attribute in method.GetCustomAttributes(true)) + switch (attribute) { - _bodyArgumentIndex = _parameters.Length - 1; - - if (_parameters.Length > 0 && - _parameters[_bodyArgumentIndex].ParameterType == typeof(CancellationToken)) - { - _bodyArgumentIndex--; - } - - if (_bodyArgumentIndex < 0) - { - throw new InvalidOperationException("No body argument found for method: " + method.Name); - } + case ContentTypeAttribute contentTypeAttribute: + _contentType = new MediaType(contentTypeAttribute.ContentType); + break; + + case ErrorResponseAttribute errorResponseAttribute: + _errorResponseType = errorResponseAttribute.Type; + break; + + case HeaderAttribute headerAttribute: + _requestHeaders.AddOrReplaceHeader(headerAttribute.Name, headerAttribute.Value, + headerAttribute.Replace); + break; + + case HttpMethodAttribute httpMethodAttribute: + templateBuilder.Append(httpMethodAttribute.Template); + _httpMethod = GetHttpMethod(httpMethodAttribute); + break; + + case PathAttribute pathAttribute: + templateBuilder.Append(pathAttribute.Template); + break; } - _serializer = context.SerializationManager.GetSerializer(_contentType); + if (IsHttpMethodWithBody()) + { + _bodyArgumentIndex = _parameters.Length - 1; - if (context.UseDomainException) + if (_parameters.Length > 0 && + _parameters[_bodyArgumentIndex].ParameterType == typeof(CancellationToken)) { - _domainExceptionMapper = context.DomainExceptionMapperFactory.CreateDomainExceptionMapper( - method, - _errorResponseType, - context.DomainExceptionType); + _bodyArgumentIndex--; } - _template = templateBuilder.ToString(); - _context = context; + if (_bodyArgumentIndex < 0) + { + throw new InvalidOperationException("No body argument found for method: " + method.Name); + } } - private bool IsHttpMethodWithBody() + _serializer = context.SerializationManager.GetSerializer(_contentType) ?? + throw new InvalidOperationException("No serializer found for content type: " + _contentType); + + if (context.DomainExceptionType != null) { - return _httpMethod == HttpMethod.Post || _httpMethod == HttpMethod.Put || _httpMethod == HttpMethod.Patch; + _domainExceptionMapper = context.DomainExceptionMapperFactory.CreateDomainExceptionMapper( + method, + _errorResponseType, + context.DomainExceptionType); } - private IParamConverter[] GetParamConverters(IParamConverterManager paramConverterManager) - { - var paramConverters = new IParamConverter[_parameters.Length]; - for (var i = 0; i < _parameters.Length; i++) - { - paramConverters[i] = paramConverterManager.GetConverter(_parameters[i].ParameterType, _parameters[i]); - } + _template = templateBuilder.ToString(); + _context = context; + } - return paramConverters; - } + private bool IsHttpMethodWithBody() + { + return _httpMethod == HttpMethod.Post || _httpMethod == HttpMethod.Put || _httpMethod == HttpMethod.Patch; + } - private static HttpMethod GetHttpMethod(HttpMethodAttribute attribute) + private IParamConverter[] GetParamConverters(IParamConverterManager paramConverterManager) + { + var paramConverters = new IParamConverter[_parameters.Length]; + for (var i = 0; i < _parameters.Length; i++) { - return attribute.HttpMethod; + paramConverters[i] = paramConverterManager.GetConverter(_parameters[i].ParameterType, _parameters[i]) + ?? throw new InvalidOperationException( + "No parameter converter found for type: " + _parameters[i].ParameterType); } - private ITaskConverter CreateConverter(RestClientContext context) + return paramConverters; + } + + private static HttpMethod GetHttpMethod(HttpMethodAttribute attribute) + { + return attribute.HttpMethod; + } + + private ITaskConverter? CreateConverter(RestClientContext context) + { + if (_actualReturnType == typeof(void)) { - return context.TaskConverterFactory.CreateTaskConverter(_actualReturnType); + return null; } - private bool IsVoidTask() + return context.TaskConverterFactory.CreateTaskConverter(_actualReturnType) ?? + throw new InvalidOperationException("Failed to create task converter for return type: " + + _actualReturnType); + } + + private bool IsVoidTask() + { + return _returnType == typeof(Task); + } + + private bool IsGenericTask() + { + return _returnType.BaseType == typeof(Task) && _returnType.IsGenericType; + } + + private Type GetActualReturnType() + { + if (IsVoidTask()) + return typeof(void); + if (IsGenericTask()) + return _returnType.GenericTypeArguments[0]; + return _returnType; + } + + private string ExpandTemplate(Dictionary routeParams) + { + var expanded = _template; + foreach (var entry in routeParams) + expanded = expanded.Replace("{" + entry.Key + "}", entry.Value.ToString()); + + return expanded; + } + + // Based on PrepareRequestMessage at https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/HttpClient.cs + private void PrepareRequestMessage(HttpRequestMessage request) + { + var baseUri = _context.BaseUri; + Uri? requestUri = null; + if (request.RequestUri == null && baseUri == null) throw new InvalidOperationException(); + if (request.RequestUri == null) { - return _returnType == typeof(Task); + requestUri = baseUri; } - - private bool IsGenericTask() + else { - return _returnType.BaseType == typeof(Task) && _returnType.IsGenericType; + // If the request Uri is an absolute Uri, just use it. Otherwise try to combine it with the base Uri. + if (!request.RequestUri.IsAbsoluteUri) + { + if (baseUri == null) + throw new InvalidOperationException(); + requestUri = new Uri(baseUri, request.RequestUri); + } } - private Type GetActualReturnType() + // We modified the original request Uri. Assign the new Uri to the request message. + if (requestUri != null) request.RequestUri = requestUri; + } + + public object? Send(object?[]? args) + { + var headers = new List>(); + headers.AddRange(_requestHeaders); + + var routeParams = new Dictionary(); + var queryParams = new List(); + var formParams = new List>(); + var partParams = new List>(); + var cancellationToken = GetParams(args, routeParams, queryParams, formParams, headers, partParams); + + var requestUriString = ExpandTemplate(routeParams); + if (queryParams.Any()) { - if (IsVoidTask()) - return typeof(void); - if (IsGenericTask()) - return _returnType.GenericTypeArguments[0]; - return _returnType; + requestUriString = requestUriString + "?" + string.Join("&", queryParams); } - private string ExpandTemplate(Dictionary routeParams) - { - var expanded = _template; - foreach (var entry in routeParams) - expanded = expanded.Replace("{" + entry.Key + "}", entry.Value.ToString()); + var requestUri = new Uri(requestUriString, UriKind.RelativeOrAbsolute); - return expanded; - } + var request = new HttpRequestMessage(_httpMethod, requestUri); - // Based on PrepareRequestMessage at https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/HttpClient.cs - private void PrepareRequestMessage(HttpRequestMessage request) + SetHeaders(request, headers); + + if (IsHttpMethodWithBody()) { - var baseUri = _context.BaseUri; - Uri requestUri = null; - if (request.RequestUri == null && baseUri == null) throw new InvalidOperationException(); - if (request.RequestUri == null) + if (partParams.Count != 0) { - requestUri = baseUri; + request.Content = CreateMultipartFormDataContent(partParams); } - else + else if (formParams.Count != 0) { - // If the request Uri is an absolute Uri, just use it. Otherwise try to combine it with the base Uri. - if (!request.RequestUri.IsAbsoluteUri) - { - if (baseUri == null) - throw new InvalidOperationException(); - requestUri = new Uri(baseUri, request.RequestUri); - } + request.Content = new FormUrlEncodedContent(formParams); + } + else if (args != null) + { + request.Content = GetHttpContent(_serializer, args[_bodyArgumentIndex], _contentType); } - - // We modified the original request Uri. Assign the new Uri to the request message. - if (requestUri != null) request.RequestUri = requestUri; } - public object Send(object[] args) - { - var headers = new List>(); - headers.AddRange(_requestHeaders); + var task = SendAsync(request, cancellationToken); - var routeParams = new Dictionary(); - var queryParams = new List(); - var formParams = new List>(); - var partParams = new List>(); - var cancellationToken = GetParams(args, routeParams, queryParams, formParams, headers, partParams); + if (IsVoidTask()) + return task; + if (_returnType.BaseType == typeof(Task) && _returnType.IsGenericType && _converter != null) + return _converter.ConvertReturnType(task); + return task.Result; + } - var requestUriString = ExpandTemplate(routeParams); - if (queryParams.Any()) + private static MultipartFormDataContent CreateMultipartFormDataContent( + IEnumerable> partParams) + { + var content = new MultipartFormDataContent(); + foreach (var part in partParams) + { + if (!string.IsNullOrEmpty(part.FileName)) { - requestUriString = requestUriString + "?" + string.Join("&", queryParams); + content.Add(part.Content, part.Name, part.FileName); } - - var requestUri = new Uri(requestUriString, UriKind.RelativeOrAbsolute); - - var request = new HttpRequestMessage(_httpMethod, requestUri); - - SetHeaders(request, headers); - - if (IsHttpMethodWithBody()) + else if (!string.IsNullOrEmpty(part.Name)) { - if (partParams.Count != 0) - { - request.Content = CreateMultipartFormDataContent(partParams); - } - else if (formParams.Count != 0) - { - request.Content = new FormUrlEncodedContent(formParams); - } - else - { - request.Content = GetHttpContent(_serializer, args[_bodyArgumentIndex], _contentType); - } + content.Add(part.Content, part.Name); } + else + { + content.Add(part.Content); + } + } - var task = SendAsync(request, cancellationToken); + return content; + } - if (IsVoidTask()) - return task; - if (_returnType.BaseType == typeof(Task) && _returnType.IsGenericType) - return _converter.ConvertReturnType(task); - return task.Result; - } + private void SetHeaders(HttpRequestMessage request, List> headers) + { + headers.ForEach(p => request.Headers.Add(p.Key, p.Value.ToString())); + } - private static MultipartFormDataContent CreateMultipartFormDataContent( - IEnumerable> partParams) - { - var content = new MultipartFormDataContent(); - foreach (var part in partParams) - { - if (!string.IsNullOrEmpty(part.FileName)) - { - content.Add(part.Content, part.Name, part.FileName); - } - else if (!string.IsNullOrEmpty(part.Name)) - { - content.Add(part.Content, part.Name); - } - else - { - content.Add(part.Content); - } - } + private string? ConvertValueToString(object? value, ParameterInfo parameterInfo) + { + if (value == null) + return null; - return content; - } + var converter = _context.ParamConverterManager.GetConverter(value.GetType(), parameterInfo); + return converter?.ToString(value) ?? value.ToString(); + } - private void SetHeaders(HttpRequestMessage request, List> headers) - { - headers.ForEach(p => request.Headers.Add(p.Key, p.Value.ToString())); - } + private CancellationToken GetParams( + object?[]? args, + Dictionary pathParams, + List queryParams, + List> formParams, + List> headers, + List> parts) + { + var cancellationToken = CancellationToken.None; - private string ConvertValueToString(object value, ParameterInfo parameterInfo) + if (_parameters.Length > 0 && args == null || _parameters.Length != args?.Length) { - if (value == null) - return null; - - var converter = _context.ParamConverterManager.GetConverter(value.GetType(), parameterInfo); - return converter?.ToString(value) ?? value.ToString(); + throw new InvalidOperationException( + $"Argument count mismatch. Expected: {_parameters.Length}, Actual: {args?.Length ?? 0}"); } - private CancellationToken GetParams( - object[] args, - Dictionary pathParams, - List queryParams, - List> formParams, - List> headers, - List> parts) + for (var i = 0; i < _parameters.Length; i++) { - var cancellationToken = CancellationToken.None; - - for (var i = 0; i < _parameters.Length; i++) + var rawValue = args[i]; + if (rawValue is CancellationToken ct) { - var rawValue = args[i]; - if (rawValue is CancellationToken ct) - { - cancellationToken = ct; - continue; - } + cancellationToken = ct; + continue; + } - var parameterAttributes = _parameters[i].GetCustomAttributes(false); - var parameterName = _parameters[i].Name; - var stringValue = _paramConverters[i].ToString(rawValue); - var handled = false; + var parameterAttributes = _parameters[i].GetCustomAttributes(false); + var parameterName = _parameters[i].Name ?? throw new InvalidOperationException( + "Parameter name not found for parameter at index: " + i); + var handled = false; - foreach (var attribute in parameterAttributes) + foreach (var attribute in parameterAttributes) + { + if (attribute is PartParamAttribute partAttribute) { - if (attribute is PartParamAttribute partAttribute) + if (_parameters[i].ParameterType.IsArray) { - if (_parameters[i].ParameterType.IsArray) + var items = (object?[]?)rawValue; + if (items != null) { - var items = (object[])rawValue; parts.AddRange(items.SelectMany(item => GetPartNameAndHttpContent(partAttribute, parameterName, item))); } - else - { - parts.AddRange(GetPartNameAndHttpContent(partAttribute, parameterName, rawValue)); - } - - handled = true; } - else if (attribute is PathParamAttribute pathParamAttribute) + else { - pathParams[pathParamAttribute.Name ?? parameterName] = Uri.EscapeDataString(stringValue); - handled = true; + parts.AddRange(GetPartNameAndHttpContent(partAttribute, parameterName, rawValue)); } - else if (attribute is QueryParamAttribute queryParamAttribute) + + handled = true; + } + else if (attribute is PathParamAttribute pathParamAttribute) + { + var stringValue = _paramConverters[i].ToString(rawValue); + pathParams[pathParamAttribute.Name ?? parameterName] = Uri.EscapeDataString(stringValue); + handled = true; + } + else if (attribute is QueryParamAttribute queryParamAttribute) + { + if (rawValue is IDictionary dictionary) { - if (rawValue is IDictionary dictionary) + foreach (DictionaryEntry entry in dictionary) { - foreach (DictionaryEntry entry in dictionary) + var key = entry.Key.ToString(); + var value = ConvertValueToString(entry.Value, _parameters[i]); + if (key != null && value != null) { - var key = entry.Key?.ToString(); - var value = ConvertValueToString(entry.Value, _parameters[i]); - if (key != null && value != null) - { - queryParams.Add(Uri.EscapeDataString(key) + "=" + Uri.EscapeDataString(value)); - } + queryParams.Add(Uri.EscapeDataString(key) + "=" + Uri.EscapeDataString(value)); } } - else if (rawValue != null) - { - queryParams.Add(Uri.EscapeDataString(queryParamAttribute.Name ?? parameterName) + "=" + - Uri.EscapeDataString(stringValue)); - } - - handled = true; } - else if (attribute is FormParamAttribute formParamAttribute) + else if (rawValue != null) { - if (rawValue is IDictionary dictionary) + var stringValue = _paramConverters[i].ToString(rawValue); + queryParams.Add(Uri.EscapeDataString(queryParamAttribute.Name ?? parameterName) + "=" + + Uri.EscapeDataString(stringValue)); + } + + handled = true; + } + else if (attribute is FormParamAttribute formParamAttribute) + { + if (rawValue is IDictionary dictionary) + { + foreach (DictionaryEntry entry in dictionary) { - foreach (DictionaryEntry entry in dictionary) + var key = entry.Key.ToString(); + var value = ConvertValueToString(entry.Value, _parameters[i]); + if (key != null && value != null) { - var key = entry.Key?.ToString(); - var value = ConvertValueToString(entry.Value, _parameters[i]); - if (key != null && value != null) - { - formParams.Add(new KeyValuePair(key, value)); - } + formParams.Add(new KeyValuePair(key, value)); } } - else if (rawValue != null) - { - formParams.Add(new KeyValuePair(formParamAttribute.Name ?? parameterName, - stringValue)); - } - - handled = true; } - else if (attribute is HeaderParamAttribute headerParamAttribute) + else if (rawValue != null) { - if (rawValue is IDictionary dictionary) + var stringValue = _paramConverters[i].ToString(rawValue); + formParams.Add(new KeyValuePair(formParamAttribute.Name ?? parameterName, + stringValue)); + } + + handled = true; + } + else if (attribute is HeaderParamAttribute headerParamAttribute) + { + if (rawValue is IDictionary dictionary) + { + foreach (DictionaryEntry entry in dictionary) { - foreach (DictionaryEntry entry in dictionary) + var key = entry.Key.ToString(); + var value = ConvertValueToString(entry.Value, _parameters[i]); + if (key != null && value != null) { - var key = entry.Key?.ToString(); - var value = ConvertValueToString(entry.Value, _parameters[i]); - if (key != null && value != null) - { - headers.AddOrReplaceHeader(key, value, headerParamAttribute.Replace); - } + headers.AddOrReplaceHeader(key, value, headerParamAttribute.Replace); } } - else if (rawValue != null) - { - headers.AddOrReplaceHeader(headerParamAttribute.Name ?? parameterName, stringValue, - headerParamAttribute.Replace); - } - - handled = true; } - } + else if (rawValue != null) + { + var stringValue = _paramConverters[i].ToString(rawValue); + headers.AddOrReplaceHeader(headerParamAttribute.Name ?? parameterName, stringValue, + headerParamAttribute.Replace); + } - if (!handled) - { - pathParams[parameterName] = Uri.EscapeDataString(stringValue); + handled = true; } } - return cancellationToken; - } - - private IEnumerable> GetPartNameAndHttpContent(PartParamAttribute partAttribute, - string parameterName, - object rawValue) - { - string fileName = null; - string partName = null; - - if (rawValue is Part part) + if (!handled) { - rawValue = part.InternalContent; - partName = part.Name; - fileName = part.FileName; - } - - if (rawValue is { }) - { - yield return new Part - { - Content = GetPartHttpContent(partAttribute, rawValue), - Name = partName ?? partAttribute.Name ?? parameterName, - FileName = fileName ?? partAttribute.FileName - }; + var stringValue = _paramConverters[i].ToString(rawValue); + pathParams[parameterName] = Uri.EscapeDataString(stringValue); } } - private HttpContent GetPartHttpContent(PartParamAttribute partAttribute, object value) - { - // TODO: prepare part serializer in advance + return cancellationToken; + } - var contentType = partAttribute.ContentType ?? DefaultPartContentType; - var serializer = _context.SerializationManager.GetSerializer(contentType); - return GetHttpContent(serializer, value, contentType); - } + private IEnumerable> GetPartNameAndHttpContent(PartParamAttribute partAttribute, + string parameterName, + object? rawValue) + { + string? fileName = null; + string? partName = null; - private static HttpContent GetHttpContent(ISerializer serializer, object value, MediaType contentType) + if (rawValue is Part part) { - if (value is HttpContent httpContent) - { - return httpContent; - } - - if (serializer == null) - { - throw new InvalidOperationException("No serializer for: " + contentType); - } - - return serializer.Serialize(value, Encoding.UTF8, contentType); + rawValue = part.InternalContent; + partName = part.Name; + fileName = part.FileName; } - - private async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + if (rawValue is { }) { - PrepareRequestMessage(request); - - if (DebugLoggingEnabled) + yield return new Part { - _context.Logger.LogDebug("{Request}", request); + Content = GetPartHttpContent(partAttribute, rawValue), + Name = partName ?? partAttribute.Name ?? parameterName, + FileName = fileName ?? partAttribute.FileName + }; + } + } - if (request.Content != null) - { - await request.Content.LoadIntoBufferAsync(); - _context.Logger.LogDebug("{RequestContent}", - (await request.Content.ReadAsStringAsync()).SafeSubstring(0, 1000)); - } - } + private HttpContent GetPartHttpContent(PartParamAttribute partAttribute, object value) + { + // TODO: prepare part serializer in advance - HttpResponseMessage response; - using (_context.RequestLogger.TimeOperation(request)) - { - response = await _context.HttpClient.SendAsync(request, cancellationToken); - } + var contentType = partAttribute.ContentType ?? DefaultPartContentType; + var serializer = _context.SerializationManager.GetSerializer(contentType) ?? + throw new InvalidOperationException("No serializer for part content type: " + contentType); + return GetHttpContent(serializer, value, contentType); + } - if (DebugLoggingEnabled) - { - _context.Logger.LogDebug("{Response}", response); + private static HttpContent GetHttpContent(ISerializer serializer, object? value, MediaType contentType) + { + if (value is HttpContent httpContent) + { + return httpContent; + } - if (response.Content != null) - { - await response.Content.LoadIntoBufferAsync(); - _context.Logger.LogDebug("{ResponseContent}", - (await response.Content.ReadAsStringAsync()).SafeSubstring(0, 1000)); - } - } + if (serializer == null) + { + throw new InvalidOperationException("No serializer for: " + contentType); + } - if (_actualReturnType == typeof(HttpStatusCode)) - { - return response.StatusCode; - } + return serializer.Serialize(value, Encoding.UTF8, contentType); + } - if (_actualReturnType == typeof(HttpResponseMessage)) - { - return response; - } - var data = await GetResponseData(request, response); + private async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + { + PrepareRequestMessage(request); - if (response.IsSuccessStatusCode) - { - return data; - } + if (DebugLoggingEnabled) + { + _context.Logger.LogDebug("{Request}", request); - if (_context.UseDomainException) + if (request.Content != null) { - throw await _domainExceptionMapper.CreateExceptionAsync(response, data); + await request.Content.LoadIntoBufferAsync(); + _context.Logger.LogDebug("{RequestContent}", + (await request.Content.ReadAsStringAsync(cancellationToken)).SafeSubstring(0, 1000)); } + } - throw new RestClientException(request.RequestUri, response.StatusCode, data); + HttpResponseMessage response; + using (_context.RequestLogger.TimeOperation(request)) + { + response = await _context.HttpClient.SendAsync(request, cancellationToken); } - private async Task GetResponseData(HttpRequestMessage request, HttpResponseMessage response) + if (DebugLoggingEnabled) { - var type = response.IsSuccessStatusCode ? _actualReturnType : _errorResponseType; + _context.Logger.LogDebug("{Response}", response); - if (type == typeof(void) || response.Content == null) - { - return null; - } + await response.Content.LoadIntoBufferAsync(); + _context.Logger.LogDebug("{ResponseContent}", + (await response.Content.ReadAsStringAsync(cancellationToken)).SafeSubstring(0, 1000)); + } - // HttpContent or a subclass like MultipartFormDataContent - if (type.IsInstanceOfType(response.Content)) - { - return response.Content; - } + if (_actualReturnType == typeof(HttpStatusCode)) + { + return response.StatusCode; + } - var contentTypeMediaType = response.Content.Headers?.ContentType?.MediaType ?? DefaultHttpContentType; - var deserializer = _context.SerializationManager.GetDeserializer(new MediaType(contentTypeMediaType)); - if (deserializer == null) - { - throw await CreateNoDeserializerFoundException(request, response, contentTypeMediaType); - } + if (_actualReturnType == typeof(HttpResponseMessage)) + { + return response; + } - try - { - return await deserializer.Deserialize(response.Content, type); - } - catch (Exception e) - { - if (e is RestClientException) - { - throw; - } + var data = await GetResponseData(request, response); - throw await CreateDeserializationException(request, response, e); - } + if (response.IsSuccessStatusCode) + { + return data; } - private async Task CreateDeserializationException(HttpRequestMessage request, - HttpResponseMessage response, Exception e) + if (_context.UseDomainException && _domainExceptionMapper != null) { - var errorResponse = response.Content == null ? null : await response.Content.ReadAsStringAsync(); + throw await _domainExceptionMapper.CreateExceptionAsync(response, data); + } - if (response.IsSuccessStatusCode || !_context.UseDomainException) - { - return new RestClientException(request.RequestUri, response.StatusCode, errorResponse, e); - } + throw new RestClientException(request.RequestUri, response.StatusCode, data); + } + + private async Task GetResponseData(HttpRequestMessage request, HttpResponseMessage response) + { + var type = response.IsSuccessStatusCode ? _actualReturnType : _errorResponseType; - return await _domainExceptionMapper.CreateExceptionAsync(response, errorResponse, e); + if (type == typeof(void)) + { + return null; } - private async Task CreateNoDeserializerFoundException(HttpRequestMessage request, - HttpResponseMessage response, - string contentTypeMediaType) + // HttpContent or a subclass like MultipartFormDataContent + if (type.IsInstanceOfType(response.Content)) { - var exception = (Exception)new RestClientException(request.RequestUri, response.StatusCode, - "No deserializer found for " + contentTypeMediaType); + return response.Content; + } + + var contentTypeMediaType = response.Content.Headers.ContentType?.MediaType ?? DefaultHttpContentType; + var deserializer = _context.SerializationManager.GetDeserializer(new MediaType(contentTypeMediaType)) ?? + throw new RestClientException(request.RequestUri, response.StatusCode, + "No deserializer found for " + contentTypeMediaType); - if (response.IsSuccessStatusCode || !_context.UseDomainException) + try + { + return await deserializer.Deserialize(response.Content, type); + } + catch (Exception e) + { + if (e is RestClientException) { - return exception; + throw; } - return await _domainExceptionMapper.CreateExceptionAsync(response, null, exception); + throw await CreateDeserializationException(request, response, e); } } + + private async Task CreateDeserializationException(HttpRequestMessage request, + HttpResponseMessage response, Exception e) + { + var errorResponse = await response.Content.ReadAsStringAsync(); + return new RestClientException(request.RequestUri, response.StatusCode, errorResponse, e); + } } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClientContext.cs b/Activout.RestClient/Implementation/RestClientContext.cs index 41d6fb0..743a00f 100644 --- a/Activout.RestClient/Implementation/RestClientContext.cs +++ b/Activout.RestClient/Implementation/RestClientContext.cs @@ -17,7 +17,7 @@ internal record RestClientContext( HttpClient HttpClient, ITaskConverterFactory TaskConverterFactory, Type? ErrorResponseType, - MediaType? DefaultContentType, + MediaType DefaultContentType, IParamConverterManager ParamConverterManager, Type? DomainExceptionType, IDomainExceptionMapperFactory DomainExceptionMapperFactory, From 1ea457f40260fa75cbf87b71d621728d9585b613 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Mon, 27 Oct 2025 17:50:58 +0100 Subject: [PATCH 5/9] Handle Coderabbit feedback Most notably dispose HttpResponseMessage if possible --- .../Implementation/RequestHandler.cs | 101 +++++++++++------- .../Implementation/RestClientBuilder.cs | 6 +- 2 files changed, 67 insertions(+), 40 deletions(-) diff --git a/Activout.RestClient/Implementation/RequestHandler.cs b/Activout.RestClient/Implementation/RequestHandler.cs index 1c910ad..76d2b26 100644 --- a/Activout.RestClient/Implementation/RequestHandler.cs +++ b/Activout.RestClient/Implementation/RequestHandler.cs @@ -39,7 +39,7 @@ internal class RequestHandler private readonly IDomainExceptionMapper? _domainExceptionMapper; private readonly List> _requestHeaders = new List>(); - private bool DebugLoggingEnabled => _context.Logger.IsEnabled(LogLevel.Debug); + private bool IsDebugLoggingEnabled => _context.Logger.IsEnabled(LogLevel.Debug); public RequestHandler(MethodInfo method, RestClientContext context) { @@ -212,7 +212,7 @@ private void PrepareRequestMessage(HttpRequestMessage request) var cancellationToken = GetParams(args, routeParams, queryParams, formParams, headers, partParams); var requestUriString = ExpandTemplate(routeParams); - if (queryParams.Any()) + if (queryParams.Count != 0) { requestUriString = requestUriString + "?" + string.Join("&", queryParams); } @@ -239,7 +239,7 @@ private void PrepareRequestMessage(HttpRequestMessage request) } } - var task = SendAsync(request, cancellationToken); + var task = SendRequestAndHandleResponse(request, cancellationToken); if (IsVoidTask()) return task; @@ -310,6 +310,11 @@ private CancellationToken GetParams( continue; } + if (rawValue == null) + { + continue; + } + var parameterAttributes = _parameters[i].GetCustomAttributes(false); var parameterName = _parameters[i].Name ?? throw new InvalidOperationException( "Parameter name not found for parameter at index: " + i); @@ -473,11 +478,17 @@ private static HttpContent GetHttpContent(ISerializer serializer, object? value, } - private async Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + private async Task SendRequestAndHandleResponse(HttpRequestMessage request, CancellationToken cancellationToken) + { + var response = await SendRequest(request, cancellationToken); + return await HandleResponse(request, response); + } + + private async Task SendRequest(HttpRequestMessage request, CancellationToken cancellationToken) { PrepareRequestMessage(request); - if (DebugLoggingEnabled) + if (IsDebugLoggingEnabled) { _context.Logger.LogDebug("{Request}", request); @@ -495,7 +506,7 @@ private static HttpContent GetHttpContent(ISerializer serializer, object? value, response = await _context.HttpClient.SendAsync(request, cancellationToken); } - if (DebugLoggingEnabled) + if (IsDebugLoggingEnabled) { _context.Logger.LogDebug("{Response}", response); @@ -504,46 +515,64 @@ private static HttpContent GetHttpContent(ISerializer serializer, object? value, (await response.Content.ReadAsStringAsync(cancellationToken)).SafeSubstring(0, 1000)); } - if (_actualReturnType == typeof(HttpStatusCode)) - { - return response.StatusCode; - } + return response; + } + private async Task HandleResponse(HttpRequestMessage request, HttpResponseMessage response) + { if (_actualReturnType == typeof(HttpResponseMessage)) { return response; } - var data = await GetResponseData(request, response); - - if (response.IsSuccessStatusCode) + var shouldDisposeResponse = true; + try { - return data; - } + if (_actualReturnType == typeof(HttpStatusCode)) + { + return response.StatusCode; + } - if (_context.UseDomainException && _domainExceptionMapper != null) - { - throw await _domainExceptionMapper.CreateExceptionAsync(response, data); - } + object? data; + var type = response.IsSuccessStatusCode ? _actualReturnType : _errorResponseType; - throw new RestClientException(request.RequestUri, response.StatusCode, data); - } + if (type == typeof(void)) + { + data = null; + } + else if (type.IsInstanceOfType(response.Content)) // HttpContent or a subclass like MultipartFormDataContent + { + shouldDisposeResponse = false; + data = response.Content; + } + else + { + data = await Deserialize(request, response, type); + } - private async Task GetResponseData(HttpRequestMessage request, HttpResponseMessage response) - { - var type = response.IsSuccessStatusCode ? _actualReturnType : _errorResponseType; + if (response.IsSuccessStatusCode) + { + return data; + } - if (type == typeof(void)) - { - return null; - } + if (_context.UseDomainException && _domainExceptionMapper != null) + { + throw await _domainExceptionMapper.CreateExceptionAsync(response, data); + } - // HttpContent or a subclass like MultipartFormDataContent - if (type.IsInstanceOfType(response.Content)) + throw new RestClientException(request.RequestUri, response.StatusCode, data); + } + finally { - return response.Content; + if (shouldDisposeResponse) + { + response.Dispose(); + } } + } + private async Task Deserialize(HttpRequestMessage request, HttpResponseMessage response, Type type) + { var contentTypeMediaType = response.Content.Headers.ContentType?.MediaType ?? DefaultHttpContentType; var deserializer = _context.SerializationManager.GetDeserializer(new MediaType(contentTypeMediaType)) ?? throw new RestClientException(request.RequestUri, response.StatusCode, @@ -560,14 +589,8 @@ private static HttpContent GetHttpContent(ISerializer serializer, object? value, throw; } - throw await CreateDeserializationException(request, response, e); + var errorResponse = await response.Content.ReadAsStringAsync(); + throw new RestClientException(request.RequestUri, response.StatusCode, errorResponse, e); } } - - private async Task CreateDeserializationException(HttpRequestMessage request, - HttpResponseMessage response, Exception e) - { - var errorResponse = await response.Content.ReadAsStringAsync(); - return new RestClientException(request.RequestUri, response.StatusCode, errorResponse, e); - } } \ No newline at end of file diff --git a/Activout.RestClient/Implementation/RestClientBuilder.cs b/Activout.RestClient/Implementation/RestClientBuilder.cs index f9273a7..292a18a 100644 --- a/Activout.RestClient/Implementation/RestClientBuilder.cs +++ b/Activout.RestClient/Implementation/RestClientBuilder.cs @@ -56,6 +56,10 @@ public IRestClientBuilder ContentType(MediaType contentType) public IRestClientBuilder Header(string name, object value, bool isReplace = true) { + if (string.Equals("content-type", name, StringComparison.OrdinalIgnoreCase)) + { + throw new InvalidOperationException("Use ContentType method to set default content type."); + } _defaultHeaders.AddOrReplaceHeader(name, value, isReplace); return this; } @@ -159,7 +163,7 @@ public T Build() where T : class HttpClient: _httpClient ?? new HttpClient(), TaskConverterFactory: _taskConverterFactory ?? TaskConverter3Factory.Instance, ErrorResponseType: _errorResponseType, - DefaultContentType: _defaultContentType, + DefaultContentType: _defaultContentType ?? throw new InvalidOperationException("DefaultContentType is not set."), ParamConverterManager: _paramConverterManager ?? ParamConverterManager.Instance, DomainExceptionType: _domainExceptionType, DomainExceptionMapperFactory: _domainExceptionMapperFactory ?? From ddd26cbc0b6cb4c03ac020c3e55a047aa619c2d7 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Mon, 27 Oct 2025 20:59:09 +0100 Subject: [PATCH 6/9] Improve test naming --- .../DomainExceptions/DomainExceptionErrorObjectTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs b/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs index c30cd36..d8d2ac5 100644 --- a/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs +++ b/Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs @@ -102,7 +102,7 @@ public async Task TestMapApiErrorObject() } [Fact] - public async Task TestNoDeserializerFound() + public async Task TestErrorResponseNotCompatibleWithHtml() { // Arrange _mockHttp From 1ce31bea763c146a38f2c6807f5e566f53e0e403 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Mon, 27 Oct 2025 22:13:05 +0100 Subject: [PATCH 7/9] Safer iteration of array parameter --- Activout.RestClient/Implementation/RequestHandler.cs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Activout.RestClient/Implementation/RequestHandler.cs b/Activout.RestClient/Implementation/RequestHandler.cs index 76d2b26..c977412 100644 --- a/Activout.RestClient/Implementation/RequestHandler.cs +++ b/Activout.RestClient/Implementation/RequestHandler.cs @@ -326,11 +326,12 @@ private CancellationToken GetParams( { if (_parameters[i].ParameterType.IsArray) { - var items = (object?[]?)rawValue; - if (items != null) + if (rawValue is IEnumerable items) { - parts.AddRange(items.SelectMany(item => - GetPartNameAndHttpContent(partAttribute, parameterName, item))); + foreach (var item in items) + { + parts.AddRange(GetPartNameAndHttpContent(partAttribute, parameterName, item)); + } } } else From 3c090cd0ac777fb058fba9ba6aecde67c7bceb2b Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Tue, 28 Oct 2025 08:26:38 +0100 Subject: [PATCH 8/9] Thank you Coderabbit! --- Activout.RestClient/Implementation/RequestHandler.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Activout.RestClient/Implementation/RequestHandler.cs b/Activout.RestClient/Implementation/RequestHandler.cs index c977412..6ad533c 100644 --- a/Activout.RestClient/Implementation/RequestHandler.cs +++ b/Activout.RestClient/Implementation/RequestHandler.cs @@ -326,7 +326,7 @@ private CancellationToken GetParams( { if (_parameters[i].ParameterType.IsArray) { - if (rawValue is IEnumerable items) + if (rawValue is IEnumerable items and not string) { foreach (var item in items) { From e25ed1ded80c65ddef688cd3a687c3290ccb9970 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Tue, 28 Oct 2025 08:47:09 +0100 Subject: [PATCH 9/9] Use braces --- Activout.RestClient/Implementation/RequestHandler.cs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Activout.RestClient/Implementation/RequestHandler.cs b/Activout.RestClient/Implementation/RequestHandler.cs index 6ad533c..2b5ae82 100644 --- a/Activout.RestClient/Implementation/RequestHandler.cs +++ b/Activout.RestClient/Implementation/RequestHandler.cs @@ -170,7 +170,9 @@ private string ExpandTemplate(Dictionary routeParams) { var expanded = _template; foreach (var entry in routeParams) + { expanded = expanded.Replace("{" + entry.Key + "}", entry.Value.ToString()); + } return expanded; }