From 2f983498b3e828cedaa957a78cd7b2bf09b761a2 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Sat, 25 Oct 2025 13:10:27 +0200 Subject: [PATCH 1/4] 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/4] 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 7d3450e3d3ad03921c286e78af282bccf16dfe98 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Sat, 25 Oct 2025 15:49:39 +0200 Subject: [PATCH 3/4] 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 38f2b4fde66c0c247adfe4929cfaef1944905031 Mon Sep 17 00:00:00 2001 From: David Eriksson Date: Sat, 25 Oct 2025 17:16:50 +0200 Subject: [PATCH 4/4] Handle more good feedback from Coderabbit --- Activout.RestClient/Implementation/RestClientBuilder.cs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/Activout.RestClient/Implementation/RestClientBuilder.cs b/Activout.RestClient/Implementation/RestClientBuilder.cs index ab953e8..f9273a7 100644 --- a/Activout.RestClient/Implementation/RestClientBuilder.cs +++ b/Activout.RestClient/Implementation/RestClientBuilder.cs @@ -91,6 +91,7 @@ public IRestClientBuilder With(IDeserializer deserializer) throw new InvalidOperationException( "Cannot add custom deserializers when a custom SerializationManager has been set."); } + _deserializers.Add(deserializer); return this; } @@ -102,6 +103,7 @@ public IRestClientBuilder With(ISerializer serializer) throw new InvalidOperationException( "Cannot add custom serializers when a custom SerializationManager has been set."); } + _serializers.Add(serializer); return this; } @@ -145,7 +147,9 @@ public T Build() where T : class _serializationManager ??= new SerializationManager( _serializers.Concat(SerializationManager.DefaultSerializers).ToList(), _deserializers.Concat(SerializationManager.DefaultDeserializers).ToList()); - _defaultSerializer ??= _serializationManager.GetSerializer(_defaultContentType) ?? StringSerializer.Instance; + _defaultSerializer ??= _serializationManager.GetSerializer(_defaultContentType) ?? + throw new InvalidOperationException( + $"No serializer found for default content type {_defaultContentType}"); var context = new RestClientContext( BaseUri: _baseUri ?? throw new InvalidOperationException("BaseUri is not set."), @@ -193,6 +197,7 @@ private void HandleAttributes(Type type) { _baseTemplate = pathAttribute.Template; } + break; } }