Remove the Services class, just create the RestClientFactory instead#123
Remove the Services class, just create the RestClientFactory instead#123
Conversation
|
Warning Rate limit exceeded@twogood has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 10 minutes and 1 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
WalkthroughThis PR refactors the factory pattern from a centralized Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Test as Test Class
participant Factory as RestClientFactory
participant Builder as RestClientBuilder
participant DT as DuckTyping
participant PCM as ParamConverterManager
participant TCF as TaskConverter3Factory
rect rgba(100, 200, 100, 0.2)
Note over Test,TCF: Old Pattern (Removed)
Test->>Services: Services.CreateRestClientFactory()
Services->>DT: CreateDuckTyping()
Services->>PCM: CreateParamConverterManager()
Services->>TCF: CreateTaskConverterFactory()
Services->>Factory: new RestClientFactory(DT, PCM, TCF)
Factory-->>Test: IRestClientFactory
end
rect rgba(100, 150, 200, 0.2)
Note over Test,TCF: New Pattern (Introduced)
Test->>Factory: new RestClientFactory()
Factory->>Builder: CreateBuilder()
Builder->>DT: DuckTyping.Instance
Builder->>PCM: ParamConverterManager.Instance
Builder->>TCF: TaskConverter3Factory.Instance
Builder-->>Factory: IRestClientBuilder
Factory-->>Test: IRestClientBuilder
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~85 minutes
Areas requiring extra attention:
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
Activout.RestClient/Implementation/RestClientBuilder.cs (1)
1-1: Enable nullable and remove global suppressionDrop “#nullable disable” and fix any warnings locally. This improves safety given the nullable-aware factories/managers.
-#nullable disable +#nullable enableActivout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs (1)
19-30: Signature mismatch with IParamConverterManager; prefer non‑null contract with fallbackGetConverter now returns nullable but the interface remains non-null; this won’t compile. Since ToStringParamConverter always matches, keep a non-null contract and remove null paths.
Apply:
*** Activout.RestClient/ParamConverter/IParamConverterManager.cs -IParamConverter GetConverter(Type type, ParameterInfo parameterInfo); +IParamConverter GetConverter(Type type, ParameterInfo parameterInfo);*** Activout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs - public IParamConverter? GetConverter(Type type, ParameterInfo parameterInfo) + public IParamConverter GetConverter(Type type, ParameterInfo parameterInfo) { - foreach (var paramConverter in ParamConverters) - { - if (paramConverter.CanConvert(type, parameterInfo)) - { - return paramConverter; - } - } - - return null; + foreach (var paramConverter in ParamConverters) + { + if (paramConverter.CanConvert(type, parameterInfo)) + { + return paramConverter; + } + } + // Fallback: should be unreachable because ToStringParamConverter.CanConvert == true + return new ToStringParamConverter(); }If your intent was to allow null, then also update IParamConverterManager to return IParamConverter? and adjust RequestHandler to handle nulls. Choose one approach; don’t mix.
🧹 Nitpick comments (12)
Activout.RestClient/RestClientFactory.cs (1)
5-11: Consider making RestClientFactory a singleton.While the current implementation is valid, since each test creates a new
RestClientFactoryinstance and the factory itself is stateless, you could optionally introduce a singleton pattern to reduce allocations. However, the current approach of direct instantiation is also acceptable and keeps the code simple.If you choose to implement this, you could add:
public class RestClientFactory : IRestClientFactory { + public static readonly RestClientFactory Instance = new(); + public IRestClientBuilder CreateBuilder() => new RestClientBuilder(); }Then tests could use
RestClientFactory.Instanceinstead ofnew RestClientFactory(), though both approaches work fine.Activout.RestClient.Test/MultipartFormDataContentTest.cs (1)
21-26: Consider using a primary constructor.The constructor could be simplified using a primary constructor pattern as specified in the coding guidelines.
As per coding guidelines.
Apply this diff:
- public MultipartFormDataContentTest(ITestOutputHelper outputHelper) + public class MultipartFormDataContentTest(ITestOutputHelper outputHelper) { - _restClientFactory = new RestClientFactory(); - _mockHttp = new MockHttpMessageHandler(); - _loggerFactory = LoggerFactoryHelpers.CreateLoggerFactory(outputHelper); - } + private readonly IRestClientFactory _restClientFactory = new RestClientFactory(); + private readonly MockHttpMessageHandler _mockHttp = new(); + private readonly ILoggerFactory _loggerFactory = LoggerFactoryHelpers.CreateLoggerFactory(outputHelper);Then remove the field declarations at lines 17-19 and move them into the class body.
Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs (1)
27-31: Consider removing the parameterless constructor.Since the constructor only initializes fields, you can use field initializers directly and remove the constructor entirely, simplifying the code.
As per coding guidelines.
Apply this diff:
- public NewtonsoftJsonDeserializerTest() - { - _restClientFactory = new RestClientFactory(); - _mockHttp = new MockHttpMessageHandler(); - } - private const string BaseUri = "https://example.com/api/"; - private readonly IRestClientFactory _restClientFactory; - private readonly MockHttpMessageHandler _mockHttp; + private readonly IRestClientFactory _restClientFactory = new RestClientFactory(); + private readonly MockHttpMessageHandler _mockHttp = new();Activout.RestClient.Test/CancellationTokenBodyTest.cs (1)
38-42: Consider removing the parameterless constructor.The constructor only initializes fields and can be replaced with field initializers for cleaner code.
As per coding guidelines.
Apply this diff:
- public CancellationTokenBodyTest() - { - _restClientFactory = new RestClientFactory(); - _mockHttp = new MockHttpMessageHandler(); - } + private readonly IRestClientFactory _restClientFactory = new RestClientFactory(); + private readonly MockHttpMessageHandler _mockHttp = new();And update lines 35-36 to remove the field declarations:
- private readonly IRestClientFactory _restClientFactory; - private readonly MockHttpMessageHandler _mockHttp;Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs (1)
75-86: Consider removing the parameterless constructor.Since the constructor only initializes fields, you can use field initializers directly and eliminate the constructor for simpler code.
As per coding guidelines.
Apply this diff:
- public DomainExceptionErrorObjectTests() - { - _mockHttp = new MockHttpMessageHandler(); - - _myApiClient = new RestClientFactory() - .CreateBuilder() - .WithNewtonsoftJson() - .With(_mockHttp.ToHttpClient()) - .BaseUri(new Uri(BaseUri)) - .With(new MyDomainExceptionMapperFactory()) - .Build<IMyApiErrorObjectClient>(); - }And update lines 72-73 to use field initializers:
- private readonly MockHttpMessageHandler _mockHttp; - private readonly IMyApiErrorObjectClient _myApiClient; + private readonly MockHttpMessageHandler _mockHttp = new(); + private readonly IMyApiErrorObjectClient _myApiClient = new RestClientFactory() + .CreateBuilder() + .WithNewtonsoftJson() + .With(new MockHttpMessageHandler().ToHttpClient()) + .BaseUri(new Uri(BaseUri)) + .With(new MyDomainExceptionMapperFactory()) + .Build<IMyApiErrorObjectClient>();Note: You'll need to adjust the
_mockHttpusage in the field initializer pattern, or keep the constructor if the initialization order matters.Activout.RestClient/Helpers/Implementation/TaskConverter3Factory.cs (1)
11-13: Avoid reflection cost in hot pathsCache constructors per T to reduce Activator.CreateInstance overhead.
Example:
-: (ITaskConverter?)Activator.CreateInstance(typeof(TaskConverter3<>).MakeGenericType(actualReturnType)); +{ + var closed = typeof(TaskConverter3<>).MakeGenericType(actualReturnType); + return (ITaskConverter)Activator.CreateInstance(closed)!; +}Or maintain a ConcurrentDictionary<Type, Func> to cache compiled factories.
Activout.RestClient/Implementation/RestClientBuilder.cs (4)
15-16: Switch to file‑scoped namespace per guidelinesAdopt file-scoped namespaces for consistency.
-namespace Activout.RestClient.Implementation -{ +namespace Activout.RestClient.Implementation; + - internal class RestClientBuilder : IRestClientBuilder - { +internal class RestClientBuilder : IRestClientBuilder +{ ... - } -} +}
21-27: Default singletons wiring LGTM; consider collection expressionsInitialization reads well. Prefer C# “[]” list syntax.
-private readonly List<ISerializer> _serializers = SerializationManager.DefaultSerializers.ToList(); -private readonly List<IDeserializer> _deserializers = SerializationManager.DefaultDeserializers.ToList(); +private readonly List<ISerializer> _serializers = [.. SerializationManager.DefaultSerializers]; +private readonly List<IDeserializer> _deserializers = [.. SerializationManager.DefaultDeserializers];
47-51: Null guard for duckTypingDefensive check avoids accidental null assignment.
- public IRestClientBuilder With(IDuckTyping duckTyping) + public IRestClientBuilder With(IDuckTyping duckTyping) { - _duckTyping = duckTyping; + _duckTyping = duckTyping ?? throw new ArgumentNullException(nameof(duckTyping)); return this; }
122-132: Minor: expression-bodied helperOptional style tweak.
-private static Uri AddTrailingSlash(Uri apiUri) -{ - var uriBuilder = new UriBuilder(apiUri ?? throw new ArgumentNullException(nameof(apiUri))); - if (uriBuilder.Path.EndsWith("/")) - { - return apiUri; - } - uriBuilder.Path = uriBuilder.Path + "/"; - return uriBuilder.Uri; -} +private static Uri AddTrailingSlash(Uri apiUri) => + (apiUri ?? throw new ArgumentNullException(nameof(apiUri))).AbsolutePath.EndsWith("/") + ? apiUri + : new UriBuilder(apiUri) { Path = (apiUri.AbsolutePath + "/") }.Uri;Activout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs (1)
15-16: Use C# “[]” collection expressionsAlign with guidelines.
-ParamConverters = new List<IParamConverter> - { new DateTimeIso8601ParamConverter(), new ToStringParamConverter() }; +ParamConverters = [ new DateTimeIso8601ParamConverter(), new ToStringParamConverter() ];Activout.RestClient/Helpers/Implementation/DuckTyping.cs (1)
9-12: Use expression‑bodied memberConcise and matches guideline.
- public TInterface DuckType<TInterface>(object originalDynamic) where TInterface : class - { - return originalDynamic.ActLike<TInterface>(); - } + public TInterface DuckType<TInterface>(object originalDynamic) where TInterface : class => + originalDynamic.ActLike<TInterface>();
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
Activout.RestClient.Json.Test/SimpleValueObjectTest.cs(1 hunks)Activout.RestClient.Json.Test/SystemTextJsonDeserializerTest.cs(1 hunks)Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.cs(1 hunks)Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorEnumTests.cs(1 hunks)Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs(1 hunks)Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs(1 hunks)Activout.RestClient.Newtonsoft.Json.Test/RestClientTests.cs(1 hunks)Activout.RestClient.Newtonsoft.Json.Test/SerializationOrderTest.cs(1 hunks)Activout.RestClient.Newtonsoft.Json.Test/SimpleValueObjectTest.cs(1 hunks)Activout.RestClient.Test.Json/RestClientTests.cs(1 hunks)Activout.RestClient.Test.Json/SerializationOrderTest.cs(1 hunks)Activout.RestClient.Test.Json/SimpleValueObjectTest.cs(1 hunks)Activout.RestClient.Test/BodyArgumentFormPostTest.cs(1 hunks)Activout.RestClient.Test/CancellationTokenBodyTest.cs(1 hunks)Activout.RestClient.Test/DictionaryParameterTests.cs(1 hunks)Activout.RestClient.Test/ErrorResponseTextPlainTest.cs(1 hunks)Activout.RestClient.Test/HttpResponseMessageTest.cs(1 hunks)Activout.RestClient.Test/HttpStatusCodeTest.cs(1 hunks)Activout.RestClient.Test/MultipartFormDataContentTest.cs(1 hunks)Activout.RestClient.Test/NonJsonRestClientTests.cs(1 hunks)Activout.RestClient.Test/NullParameterTests.cs(1 hunks)Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs(1 hunks)Activout.RestClient.Xml.Test/XmlTests.cs(1 hunks)Activout.RestClient/Helpers/Implementation/DuckTyping.cs(1 hunks)Activout.RestClient/Helpers/Implementation/TaskConverter3Factory.cs(1 hunks)Activout.RestClient/Implementation/RestClientBuilder.cs(3 hunks)Activout.RestClient/Implementation/RestClientFactory.cs(0 hunks)Activout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs(1 hunks)Activout.RestClient/RestClientFactory.cs(1 hunks)Activout.RestClient/Services.cs(0 hunks)
💤 Files with no reviewable changes (2)
- Activout.RestClient/Services.cs
- Activout.RestClient/Implementation/RestClientFactory.cs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.cs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.cs: Use [] list syntax for collections
Use file-scoped namespaces
Use primary constructors wherever possible
Use records for data transfer objects (DTOs) and immutable data structures
Use var for local variable declarations when possible
Use expression-bodied members for simple methods and properties
Use pattern matching for type checks and deconstruction
Use using statements for resource management
Use async and await for asynchronous programming
Never include "Async" in method names
Only include comments that explain why something is done, not what is done
Files:
Activout.RestClient.Test/CancellationTokenBodyTest.csActivout.RestClient.Test/HttpResponseMessageTest.csActivout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.csActivout.RestClient/RestClientFactory.csActivout.RestClient.Test/NullParameterTests.csActivout.RestClient.Json.Test/SystemTextJsonDeserializerTest.csActivout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorEnumTests.csActivout.RestClient.Newtonsoft.Json.Test/RestClientTests.csActivout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.csActivout.RestClient.Xml.Test/XmlTests.csActivout.RestClient.Test/NonJsonRestClientTests.csActivout.RestClient.Test/MultipartFormDataContentTest.csActivout.RestClient.Xml.Test/ErrorResponseXmlTest.csActivout.RestClient/Helpers/Implementation/TaskConverter3Factory.csActivout.RestClient.Test/HttpStatusCodeTest.csActivout.RestClient.Test/DictionaryParameterTests.csActivout.RestClient.Newtonsoft.Json.Test/SimpleValueObjectTest.csActivout.RestClient/Implementation/RestClientBuilder.csActivout.RestClient.Test.Json/SimpleValueObjectTest.csActivout.RestClient.Test/ErrorResponseTextPlainTest.csActivout.RestClient.Test/BodyArgumentFormPostTest.csActivout.RestClient.Test.Json/SerializationOrderTest.csActivout.RestClient.Test.Json/RestClientTests.csActivout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.csActivout.RestClient/ParamConverter/Implementation/ParamConverterManager.csActivout.RestClient.Newtonsoft.Json.Test/SerializationOrderTest.csActivout.RestClient.Json.Test/SimpleValueObjectTest.csActivout.RestClient/Helpers/Implementation/DuckTyping.cs
🧬 Code graph analysis (28)
Activout.RestClient.Test/CancellationTokenBodyTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test/HttpResponseMessageTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient/RestClientFactory.cs (1)
Activout.RestClient/Implementation/RestClientBuilder.cs (2)
IRestClientBuilder(29-33)RestClientBuilder(17-133)
Activout.RestClient.Test/NullParameterTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Json.Test/SystemTextJsonDeserializerTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorEnumTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Newtonsoft.Json.Test/RestClientTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Xml.Test/XmlTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test/NonJsonRestClientTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test/MultipartFormDataContentTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient/Helpers/Implementation/TaskConverter3Factory.cs (5)
Activout.RestClient/Implementation/RequestHandler.cs (2)
ITaskConverter(136-139)Type(151-158)Activout.RestClient/Helpers/ITaskConverterFactory.cs (1)
ITaskConverter(8-8)Activout.RestClient/Helpers/Implementation/TaskConverter2Factory.cs (1)
ITaskConverter(9-12)Activout.RestClient/Helpers/Implementation/TaskConverterFactory.cs (1)
ITaskConverter(9-12)Activout.RestClient/Helpers/Implementation/TaskConverter3.cs (1)
TaskConverter3(10-23)
Activout.RestClient.Test/HttpStatusCodeTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test/DictionaryParameterTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Newtonsoft.Json.Test/SimpleValueObjectTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient/Implementation/RestClientBuilder.cs (5)
Activout.RestClient/Helpers/Implementation/DuckTyping.cs (1)
DuckTyping(5-13)Activout.RestClient/Implementation/RestClientContext.cs (2)
RestClientContext(16-46)RestClientContext(23-29)Activout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs (2)
ParamConverterManager(7-31)ParamConverterManager(13-17)Activout.RestClient/Helpers/Implementation/TaskConverter3Factory.cs (1)
TaskConverter3Factory(5-15)Activout.RestClient/IRestClientBuilder.cs (11)
IRestClientBuilder(13-13)IRestClientBuilder(14-14)IRestClientBuilder(15-15)IRestClientBuilder(16-16)IRestClientBuilder(17-17)IRestClientBuilder(18-18)IRestClientBuilder(19-19)IRestClientBuilder(20-20)IRestClientBuilder(21-21)IRestClientBuilder(22-22)IRestClientBuilder(23-23)
Activout.RestClient.Test.Json/SimpleValueObjectTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test/ErrorResponseTextPlainTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test/BodyArgumentFormPostTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test.Json/SerializationOrderTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Test.Json/RestClientTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs (4)
Activout.RestClient/Implementation/RequestHandler.cs (2)
IParamConverter(120-129)Type(151-158)Activout.RestClient/ParamConverter/IParamConverterManager.cs (1)
IParamConverter(9-9)Activout.RestClient/ParamConverter/Implementation/DateTimeIso8601ParamConverter.cs (1)
DateTimeIso8601ParamConverter(7-18)Activout.RestClient/ParamConverter/Implementation/ToStringParamConverter.cs (1)
ToStringParamConverter(7-18)
Activout.RestClient.Newtonsoft.Json.Test/SerializationOrderTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient.Json.Test/SimpleValueObjectTest.cs (1)
Activout.RestClient/RestClientFactory.cs (1)
RestClientFactory(5-11)
Activout.RestClient/Helpers/Implementation/DuckTyping.cs (1)
Activout.RestClient/Helpers/IDuckTyping.cs (1)
TInterface(6-6)
🔇 Additional comments (26)
Activout.RestClient.Test/BodyArgumentFormPostTest.cs (1)
42-42: LGTM! Clean migration to direct factory instantiation.The change from
Services.CreateRestClientFactory()tonew RestClientFactory()correctly implements the PR's goal of removing the centralized Services class.Activout.RestClient.Xml.Test/XmlTests.cs (1)
12-12: LGTM! Consistent with the factory refactoring.The direct instantiation of
RestClientFactoryaligns with the PR's refactoring goals. The file already follows coding guidelines with primary constructors and file-scoped namespaces.Activout.RestClient.Test/HttpStatusCodeTest.cs (1)
19-19: LGTM! Consistent factory instantiation.The migration from the Services factory method to direct instantiation is implemented correctly.
Activout.RestClient.Newtonsoft.Json.Test/RestClientTests.cs (1)
21-21: LGTM! Factory refactoring applied correctly.The change successfully removes the dependency on the Services class.
Activout.RestClient.Json.Test/SimpleValueObjectTest.cs (1)
29-29: LGTM! Proper factory instantiation.The direct instantiation pattern is correctly applied.
Activout.RestClient.Test.Json/SimpleValueObjectTest.cs (1)
31-31: LGTM! Consistent with refactoring pattern.The factory instantiation follows the same pattern established across other test files.
Activout.RestClient.Newtonsoft.Json.Test/SerializationOrderTest.cs (1)
23-23: LGTM! Factory migration implemented correctly.The change properly removes the Services class dependency.
Activout.RestClient.Xml.Test/ErrorResponseXmlTest.cs (1)
13-13: LGTM! Clean factory instantiation.The change from
Services.CreateRestClientFactory()tonew RestClientFactory()is correct and aligns with the PR objective to remove the Services class.Activout.RestClient.Newtonsoft.Json.Test/SimpleValueObjectTest.cs (1)
41-41: LGTM! Correct factory instantiation.The direct instantiation of
RestClientFactoryproperly replaces the previous service-based approach.Activout.RestClient.Test/MultipartFormDataContentTest.cs (1)
23-23: LGTM! Correct factory instantiation.The change to directly instantiate
RestClientFactoryis correct and consistent with the PR objectives.Activout.RestClient.Test/ErrorResponseTextPlainTest.cs (1)
15-15: LGTM! Correct factory instantiation.The direct instantiation of
RestClientFactoryproperly replaces the service-based factory pattern.Activout.RestClient.Newtonsoft.Json.Test/NewtonsoftJsonDeserializerTest.cs (1)
29-29: LGTM! Correct factory instantiation.The change to
new RestClientFactory()is correct and consistent with the PR objective.Activout.RestClient.Test/CancellationTokenBodyTest.cs (1)
40-40: LGTM! Correct factory instantiation.The direct instantiation of
RestClientFactoryis correct and aligns with the PR's refactoring goals.Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorObjectTests.cs (1)
79-86: LGTM! Correct factory instantiation.The inline usage of
new RestClientFactory().CreateBuilder()is correct and concise, properly replacing the previous service-based approach.Activout.RestClient.Test.Json/RestClientTests.cs (1)
35-35: LGTM! Correct factory instantiation.The change to directly instantiate
RestClientFactoryis correct and consistent with the broader refactoring to remove the Services class.Activout.RestClient.Json.Test/SystemTextJsonDeserializerTest.cs (1)
24-24: LGTM! Clean refactor from Services indirection to direct instantiation.This change successfully removes the dependency on the centralized
Servicesclass in favor of direct factory instantiation, which simplifies the code and makes dependencies more explicit.Activout.RestClient.Test/DictionaryParameterTests.cs (1)
17-17: LGTM! Consistent with the factory refactor pattern.Activout.RestClient.Test/NullParameterTests.cs (1)
15-15: LGTM! Consistent with the factory refactor pattern.Activout.RestClient.Test/HttpResponseMessageTest.cs (1)
19-19: LGTM! Consistent with the factory refactor pattern.Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DomainExceptionErrorEnumTests.cs (1)
47-47: LGTM! Consistent with the factory refactor pattern.The inline instantiation within the fluent chain is a valid approach and maintains code clarity.
Activout.RestClient.Test.Json/SerializationOrderTest.cs (1)
19-19: LGTM! Consistent with the factory refactor pattern.Activout.RestClient.Newtonsoft.Json.Test/DomainExceptions/DefaultDomainExceptionErrorObjectTests.cs (1)
44-44: LGTM! Consistent with the factory refactor pattern.Activout.RestClient.Test/NonJsonRestClientTests.cs (1)
23-23: LGTM! Consistent with the factory refactor pattern.Activout.RestClient/Helpers/Implementation/TaskConverter3Factory.cs (1)
7-7: Singleton accessor looks goodThread-safe static initialization; consistent with the new pattern used elsewhere.
Please confirm all previous creation paths were updated to use TaskConverter3Factory.Instance.
Activout.RestClient/ParamConverter/Implementation/ParamConverterManager.cs (1)
9-10: Singleton accessor looks goodNo concerns.
Activout.RestClient/Helpers/Implementation/DuckTyping.cs (1)
5-8: LGTM on singleton and file‑scoped namespaceMatches the new pattern; no issues.
Rebase failed
Summary by CodeRabbit
Refactor
Tests