refactor: make sub-clients internal, only ITorBoxClient exposed via DI#13
Conversation
…ia DI - All concrete client classes (TorrentsClient, UsenetClient, etc.) are now internal - MainApiClient directly instantiates all resource clients from a shared HttpClient - TorBoxClient uses IHttpClientFactory with named clients to create sub-clients - DI only registers ITorBoxClient; sub-clients are not in the container - Added HttpClientNames for named HTTP client constants - Updated integration tests to resolve sub-clients through ITorBoxClient - Added InternalsVisibleTo for TorBoxSDK.IntegrationTests Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/44c680ba-a822-4478-a615-953e1c430e30 Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/44c680ba-a822-4478-a615-953e1c430e30 Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
|
@copilot assure toi que les tests sont à jours et que la documentation est également à jour avec ce nouveau système. Pour le moment le SDK n'est pas publique, donc il n'y a aucune notion de breaking changes, nous sommes encore en phase de développement. Il faudra également mettre à jour les agents, instructions et skills personnalisés pour refléter ce changement important (si nécessaire). |
- Add unit tests for sub-clients not directly registered in DI - Add unit test verifying all Main resource clients accessible through ITorBoxClient - Update ClientTestBase error message to reflect internal constructors - Update docs/architecture.md with DI and internal sub-clients explanation - Update docs/configuration.md to remove mention of individual sub-client registration - Update csharp-conventions.instructions.md DI section for ITorBoxClient-only registration - Update dev-jobs.md J6 Foundation DI checklist to reflect new pattern Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/9d7790cc-94a2-4ca7-9881-04d7a2ef47b7 Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
Done in Tests mis à jour :
Documentation mise à jour :
Instructions et skills mis à jour :
|
|
@copilot convertis toute la documentation en anglais + les skills / instructions / agents. Absolument tout doit être en anglais dans le repo. |
Translate all French prose and descriptions to English while preserving all Markdown formatting, code blocks, technical terms, file paths, and structural elements (J1-J6 job definitions, checklists, tables, etc.). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
Translate all French prose and descriptions to English while preserving all Markdown formatting, code blocks, file paths, and technical terms. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
… to English - dev-jobs.md: full translation (J1-J6 definitions, checklists, exit criteria) - development-playbooks.md: full translation (Playbooks A-F, handoff rules) - skill-map.md: full translation (decision table, priority rules, agent list) - plan-torBoxSdkCorrections.prompt.md: full translation (5-phase SDK correction plan) - code-review/SKILL.md: remove French trigger words from description - code-reviewer.agent.md: remove French trigger words from description Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/040f9299-d89e-4830-b7ba-8a3309febe4a Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
All French content has been translated to English across commits Full translations (French → English):
French trigger words removed:
All other files (agents, instructions, docs, remaining skills and references) were already in English. Zero French characters remain in the repository. |
There was a problem hiding this comment.
Pull request overview
Refactors the SDK’s DI surface so consumers resolve a single ITorBoxClient, with concrete sub-clients made internal and instantiated internally, plus associated test and documentation updates.
Changes:
- Make concrete API clients/resource clients
internaland remove sub-client registrations from DI; expose onlyITorBoxClient. - Update
AddTorBox()to register namedHttpClients and constructTorBoxClientinternally viaIHttpClientFactory. - Update unit/integration tests and docs to reflect the new DI/instantiation approach; translate various repo reference docs to English.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/TorboxSDK.UnitTests/Helpers/ClientTestBase.cs | Updates helper messaging for internal constructors (but helper creation path needs adjustment). |
| tests/TorboxSDK.UnitTests/DependencyInjection/TorBoxServiceCollectionExtensionsTests.cs | Adjusts DI expectations/tests to only resolve ITorBoxClient. |
| tests/TorBoxSDK.IntegrationTests/DependencyInjection/AddTorBoxIntegrationTests.cs | Updates integration DI tests to use ITorBoxClient and asserts sub-clients aren’t registered. |
| src/TorBoxSDK/TorBoxClient.cs | Reworks root client construction to use IHttpClientFactory + named clients. |
| src/TorBoxSDK/Search/SearchApiClient.cs | Makes concrete Search API client internal. |
| src/TorBoxSDK/Relay/RelayApiClient.cs | Makes concrete Relay API client internal. |
| src/TorBoxSDK/Properties/AssemblyInfo.cs | Exposes internals to integration tests. |
| src/TorBoxSDK/Main/WebDownloads/WebDownloadsClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/Vendors/VendorsClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/User/UserClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/Usenet/UsenetClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/Torrents/TorrentsClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/Stream/StreamClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/Rss/RssClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/Queued/QueuedClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/Notifications/NotificationsClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/MainApiClient.cs | Makes Main API aggregate client internal and instantiates resource clients internally from a shared HttpClient. |
| src/TorBoxSDK/Main/Integrations/IntegrationsClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Main/General/GeneralClient.cs | Makes concrete client internal. |
| src/TorBoxSDK/Http/HttpClientNames.cs | Adds internal constants for named HttpClient registrations. |
| src/TorBoxSDK/DependencyInjection/TorBoxServiceCollectionExtensions.cs | Simplifies DI: registers named clients + ITorBoxClient only. |
| docs/configuration.md | Documents the new “resolve ITorBoxClient only” DI guidance. |
| docs/architecture.md | Adds DI/instantiation section explaining internal sub-clients and access through ITorBoxClient. |
| Directory.Build.props | Updates package metadata (including version). |
| .github/skills/dev/references/skill-map.md | Translates and updates internal repo guidance text. |
| .github/skills/dev/references/development-playbooks.md | Translates and updates internal repo guidance text. |
| .github/skills/dev/references/dev-jobs.md | Translates and updates internal repo guidance text, including DI checklist notes. |
| .github/skills/code-review/SKILL.md | Removes French trigger words from description. |
| .github/prompts/plan-torBoxSdkCorrections.prompt.md | Translates plan prompt content to English. |
| .github/instructions/csharp-conventions.instructions.md | Updates DI guidance to reflect only ITorBoxClient registration. |
| .github/agents/code-reviewer.agent.md | Removes French trigger words from description. |
| HttpClient httpClient = new(handler) { BaseAddress = new Uri(resolvedBaseAddress) }; | ||
|
|
||
| TClient client = (TClient?)Activator.CreateInstance(typeof(TClient), httpClient) | ||
| ?? throw new InvalidOperationException($"Failed to create instance of {typeof(TClient).Name}. Ensure it has a public constructor accepting HttpClient."); | ||
| ?? throw new InvalidOperationException($"Failed to create instance of {typeof(TClient).Name}. Ensure it has an internal constructor accepting HttpClient."); | ||
|
|
There was a problem hiding this comment.
Activator.CreateInstance(typeof(TClient), httpClient) only binds to public constructors. Since the SDK sub-client constructors are now internal, this call will return null and the helper will always throw. Use an overload that allows non-public constructors (or invoke the HttpClient ctor via reflection with appropriate BindingFlags) so unit tests can still construct internal clients.
There was a problem hiding this comment.
Fixed in bb1be47. ClientTestBase.CreateClient<TClient> now uses Activator.CreateInstance with BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic to explicitly bind to internal constructors. All 323 unit tests pass.
| /// <para> | ||
| /// Register the client through dependency injection using | ||
| /// <see cref="DependencyInjection.TorBoxServiceCollectionExtensions.AddTorBox(Microsoft.Extensions.DependencyInjection.IServiceCollection, System.Action{TorBoxClientOptions})"/> | ||
| /// or construct it directly for non-DI scenarios. | ||
| /// </para> | ||
| /// </remarks> | ||
| /// <remarks> | ||
| /// Initializes a new instance of the <see cref="TorBoxClient"/> class. | ||
| /// </remarks> | ||
| /// <param name="main">The Main API client.</param> | ||
| /// <param name="search">The Search API client.</param> | ||
| /// <param name="relay">The Relay API client.</param> | ||
| /// <exception cref="ArgumentNullException"> | ||
| /// Thrown when <paramref name="main"/>, <paramref name="search"/>, or <paramref name="relay"/> is <see langword="null"/>. | ||
| /// </exception> | ||
| public sealed class TorBoxClient(IMainApiClient main, ISearchApiClient search, IRelayApiClient relay) : ITorBoxClient | ||
| public sealed class TorBoxClient : ITorBoxClient | ||
| { | ||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="TorBoxClient"/> class | ||
| /// using an <see cref="IHttpClientFactory"/> to create the required HTTP clients. | ||
| /// </summary> | ||
| /// <param name="httpClientFactory">The HTTP client factory used to create named clients.</param> | ||
| /// <param name="options">The SDK configuration options.</param> | ||
| /// <exception cref="ArgumentNullException"> | ||
| /// Thrown when <paramref name="httpClientFactory"/> or <paramref name="options"/> is <see langword="null"/>. | ||
| /// </exception> | ||
| internal TorBoxClient(IHttpClientFactory httpClientFactory, IOptions<TorBoxClientOptions> options) | ||
| { |
There was a problem hiding this comment.
The XML docs say consumers can "construct it directly for non-DI scenarios", but the only constructor is now internal, so external callers cannot instantiate TorBoxClient outside DI. Either update the docs to remove/replace that guidance, or provide a public construction path (e.g., a public factory/constructor intended for non-DI usage).
There was a problem hiding this comment.
Fixed in 7d3514d. Updated the XML docs on TorBoxClient to remove the misleading "construct it directly for non-DI scenarios" guidance. The docs now state that the class cannot be instantiated directly and is created internally by the DI container.
Remove misleading guidance about direct construction for non-DI scenarios. The constructor is internal, so TorBoxClient can only be created through DI. Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/1f92c752-5423-4af6-8545-12171d55c3ec Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
…uctors Activator.CreateInstance(Type, object[]) only binds to public constructors. Use the BindingFlags overload with NonPublic to explicitly support internal constructors, making the test helper robust against the sub-client visibility refactor. Agent-Logs-Url: https://github.com/devRael1/TorBoxSDK/sessions/a59ed878-55d6-4a2d-8e38-2f384e27c389 Co-authored-by: devRael1 <91017912+devRael1@users.noreply.github.com>
All tests are up to date: 323 unit tests pass, 43 integration tests (4 pass, 39 skip without API key). Documentation, instructions, and skills were updated in previous commits (
All content was translated to English in commits |
|
@copilot assure toi que le projet est utilisable dans tous les framework de .NET 6 à .NET 10. |
Verified — the SDK already targets |
internalinstead ofpublicITorBoxClientwith named HttpClientsTorBoxClientandMainApiClientto instantiate sub-clients internallyClientTestBase.csto useBindingFlags.NonPublicfor internal constructors