feat: Add connection string provider #1588
Conversation
✅ Deploy Preview for testcontainers-dotnet ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughAdds a Connection String Provider API and integrates it into builder/configuration/container flows: new interfaces, enum, adapter, configuration property, builder fluent API, container wiring and GetConnectionString methods, docs, and tests. Changes
Sequence Diagram(s)sequenceDiagram
participant Builder as ContainerBuilder
participant Config as ContainerConfiguration
participant Docker as DockerContainer
participant Provider as IConnectionStringProvider
Builder->>Builder: WithConnectionStringProvider(provider)
Builder->>Config: Attach provider (wrap generic -> adapter)
Builder->>Docker: Build & StartAsync()
Docker->>Config: Read ConnectionStringProvider
alt provider configured
Docker->>Provider: Configure(container, configuration)
Note over Docker,Provider: Provider stored on container for runtime
else none configured
Docker-->>Docker: No provider (will throw on Get)
end
Docker->>Provider: GetConnectionString(mode/name)
Provider-->>Docker: Return connection string (or throw if not configured)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
src/Testcontainers/Configurations/Containers/ConnectionMode.cs (1)
5-20: Enum shape and placement look good; optional doc tweakThe
ConnectionModeenum is minimal and well‑placed inDotNet.Testcontainers.Configurations, and theContainer/Hostvalues map cleanly to the intended usage inGetConnectionString(...).If you want to be extra explicit for consumers, you could slightly rephrase the summaries to make the perspective clear, e.g.:
- /// <summary> - /// The connection string for the container. - /// </summary> + /// <summary> + /// Resolve a connection string usable from within a container (e.g. using container hostname/alias). + /// </summary>and similarly for
Host.src/Testcontainers/Configurations/Containers/IContainerConfiguration.cs (1)
129-132: New property is reasonable, but clarify nullability and consider compatibility impactThe
ConnectionStringProviderhook fits well onIContainerConfigurationand matches the builder/container wiring.Two follow‑ups to consider:
Document null semantics
In many configurations a provider won’t be set. To make that clear to consumers, you could adjust the XML doc to signal thatnullis a valid value, e.g.:
- ///
- /// Gets the connection string provider.
- ///
- IConnectionStringProvider<IContainer, IContainerConfiguration> ConnectionStringProvider { get; }
- ///
- /// Gets the connection string provider, or if none is configured.
- ///
- IConnectionStringProvider<IContainer, IContainerConfiguration> ConnectionStringProvider { get; }
2. **Public interface expansion is breaking** Adding this property to `IContainerConfiguration` is a source/binary breaking change for any external implementations of this interface. Please double‑check that this is acceptable for the targeted release (e.g., next major), or that downstream implementers are expected to be updated accordingly. If not, an alternative could be a separate interface (e.g. `IConnectionStringConfig`) plus extension methods. </blockquote></details> <details> <summary>src/Testcontainers/Containers/ConnectionStringProviderNotConfiguredException.cs (1)</summary><blockquote> `6-19`: **Exception is fine; consider additional constructors for future extensibility** The dedicated `ConnectionStringProviderNotConfiguredException` with a clear default message works well for the current use case. If you expect this to be used in more complex flows later, you might optionally add the standard overloads (message, message+inner) to make wrapping easier, but it’s not required for this PR. </blockquote></details> <details> <summary>src/Testcontainers/Builders/ContainerBuilder`3.cs (1)</summary><blockquote> `399-403`: **Wiring looks correct; add a null guard to avoid confusing NREs** The new `WithConnectionStringProvider` correctly adapts the strongly‑typed provider into the non‑generic `ConnectionStringProvider` used by the configuration. One edge case: if someone accidentally calls `WithConnectionStringProvider(null)`, they’ll end up with an adapter whose inner provider is `null`, and the first call to `GetConnectionString` will likely blow up with a `NullReferenceException` instead of the explicit `ConnectionStringProviderNotConfiguredException`. Consider failing fast on misuse: ```diff public TBuilderEntity WithConnectionStringProvider(IConnectionStringProvider<TContainerEntity, TConfigurationEntity> connectionStringProvider) { + if (connectionStringProvider is null) + { + throw new ArgumentNullException(nameof(connectionStringProvider)); + } + return Clone( new ContainerConfiguration( connectionStringProvider: new ConnectionStringProvider<TContainerEntity, TConfigurationEntity>(connectionStringProvider))); }This keeps the happy path unchanged but makes errors clearer when the API is used incorrectly.
tests/Testcontainers.Platform.Linux.Tests/ConnectionStringProviderTest.cs (1)
32-38: Consider adding test coverage for error cases and ConnectionMode.Container.The current test validates the happy path well. Consider adding tests for:
GetConnectionStringwithout a provider configured (should throwConnectionStringProviderNotConfiguredException)GetConnectionString(ConnectionMode.Container)to verify the mode parameter is passed correctlysrc/Testcontainers/Configurations/Containers/IConnectionStringProvider.cs (1)
21-29: Consider documenting behavior for unknown connection string names.The
GetConnectionString(string name, ...)overload documents that it throws when the provider is not configured, but doesn't specify the behavior when an unknown name is requested. Consider adding documentation or a dedicated exception (e.g.,ConnectionStringNotFoundException) for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
docs/api/connection_string_provider.md(1 hunks)mkdocs.yml(1 hunks)src/Testcontainers.Couchbase/CouchbaseBuilder.cs(1 hunks)src/Testcontainers/Builders/ContainerBuilder3.cs` (1 hunks)src/Testcontainers/Builders/IContainerBuilder2.cs` (2 hunks)src/Testcontainers/Configurations/Containers/ConnectionMode.cs(1 hunks)src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs(1 hunks)src/Testcontainers/Configurations/Containers/ContainerConfiguration.cs(4 hunks)src/Testcontainers/Configurations/Containers/IConnectionStringProvider.cs(1 hunks)src/Testcontainers/Configurations/Containers/IConnectionStringProvider2.cs` (1 hunks)src/Testcontainers/Configurations/Containers/IContainerConfiguration.cs(1 hunks)src/Testcontainers/Configurations/WaitStrategies/IWaitUntil.cs(1 hunks)src/Testcontainers/Configurations/WaitStrategies/IWaitWhile.cs(1 hunks)src/Testcontainers/Containers/ConnectionStringProviderNotConfiguredException.cs(1 hunks)src/Testcontainers/Containers/DockerContainer.cs(3 hunks)src/Testcontainers/Containers/IContainer.cs(1 hunks)tests/Testcontainers.Platform.Linux.Tests/ConnectionStringProviderTest.cs(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-17T17:58:43.958Z
Learnt from: diegosasw
Repo: testcontainers/testcontainers-dotnet PR: 1583
File: src/Testcontainers.KurrentDb/Testcontainers.KurrentDb.csproj:7-7
Timestamp: 2025-11-17T17:58:43.958Z
Learning: In the testcontainers-dotnet repository, JetBrains.Annotations should use version 2023.3.0 to maintain consistency with the main Testcontainers csproj, rather than always using the latest available version.
Applied to files:
src/Testcontainers/Containers/IContainer.cs
🧬 Code graph analysis (8)
src/Testcontainers/Containers/IContainer.cs (1)
src/Testcontainers/Builders/ContainerBuilder.cs (1)
IContainer(54-58)
src/Testcontainers/Builders/ContainerBuilder`3.cs (2)
src/Testcontainers/Configurations/Containers/ContainerConfiguration.cs (4)
ContainerConfiguration(44-92)ContainerConfiguration(98-101)ContainerConfiguration(107-110)ContainerConfiguration(117-143)src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (2)
ConnectionStringProvider(6-38)ConnectionStringProvider(16-19)
src/Testcontainers/Builders/IContainerBuilder`2.cs (1)
src/Testcontainers/Builders/ContainerBuilder`3.cs (16)
TBuilderEntity(55-59)TBuilderEntity(62-66)TBuilderEntity(69-72)TBuilderEntity(75-78)TBuilderEntity(81-84)TBuilderEntity(87-90)TBuilderEntity(93-96)TBuilderEntity(99-102)TBuilderEntity(105-108)TBuilderEntity(111-114)TBuilderEntity(117-120)TBuilderEntity(123-126)TBuilderEntity(129-132)TBuilderEntity(135-139)TBuilderEntity(142-145)TBuilderEntity(148-152)
tests/Testcontainers.Platform.Linux.Tests/ConnectionStringProviderTest.cs (1)
src/Testcontainers/Builders/ContainerBuilder.cs (1)
IContainer(54-58)
src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (2)
src/Testcontainers/Configurations/Containers/IConnectionStringProvider`2.cs (1)
Configure(17-17)src/Testcontainers/Containers/DockerContainer.cs (2)
GetConnectionString(422-430)GetConnectionString(433-441)
src/Testcontainers/Configurations/Containers/IConnectionStringProvider`2.cs (2)
src/Testcontainers/Builders/ContainerBuilder.cs (1)
IContainer(54-58)src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (1)
Configure(22-25)
src/Testcontainers/Configurations/Containers/IContainerConfiguration.cs (1)
src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (2)
ConnectionStringProvider(6-38)ConnectionStringProvider(16-19)
src/Testcontainers/Configurations/Containers/IConnectionStringProvider.cs (3)
src/Testcontainers/Containers/ConnectionStringProviderNotConfiguredException.cs (1)
PublicAPI(9-19)src/Testcontainers/Containers/DockerContainer.cs (3)
PublicAPI(19-789)GetConnectionString(422-430)GetConnectionString(433-441)src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (2)
GetConnectionString(28-31)GetConnectionString(34-37)
🪛 LanguageTool
docs/api/connection_string_provider.md
[style] ~4-~4: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 750 characters long)
Context: ... API base addresses) in a uniform way. !!!note Testcontainers modules do not ...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
docs/api/connection_string_provider.md
13-13: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
34-34: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
60-60: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: analyze (csharp)
🔇 Additional comments (18)
src/Testcontainers.Couchbase/CouchbaseBuilder.cs (1)
179-185: Doc comment clarification forcontainerparameter looks goodThe updated XML documentation explicitly stating "The Couchbase container." is clearer and aligns with how
containeris used throughoutConfigureCouchbaseAsync; no further changes needed here.mkdocs.yml (1)
43-43: Connection string provider docs entry correctly wired into navThe new
api/connection_string_provider.mdentry is placed consistently within the API section; no issues from a docs/config standpoint.src/Testcontainers/Configurations/WaitStrategies/IWaitWhile.cs (1)
16-16: Doc wording matches generic wait semanticsThe updated parameter description (“check the condition for”) better reflects
IWaitWhile’s purpose and stays consistent withIWaitUntil; no functional impact.src/Testcontainers/Configurations/WaitStrategies/IWaitUntil.cs (1)
16-16: Consistent documentation across wait strategy interfacesThe revised parameter description is clearer and aligns with
IWaitWhile; looks good.src/Testcontainers/Containers/IContainer.cs (1)
17-17: Extending IContainer with IConnectionStringProvider is a breaking API surface changeHaving
IContainerimplementIConnectionStringProvideris a nice ergonomic win (container.GetConnectionString(...)), but it also means:
- Any custom
IContainerimplementations outside this repo now have to implement the provider contract.- Existing compiled code against the old interface may break at runtime once upgraded.
Please validate that this is intended for the release train this PR targets (e.g., planned major version) and, if not, consider alternatives such as:
- A separate interface (e.g.
IConnectionStringAwareContainer) implemented byDockerContainerand module containers, plus- Extension methods on
IContainerthat safely downcast or throw a clearer error when the provider isn’t supported.src/Testcontainers/Builders/IContainerBuilder`2.cs (2)
24-25: LGTM!The added generic constraints properly enforce type safety for the container and configuration entities, which enables the strongly-typed connection string provider pattern.
505-512: LGTM!The new
WithConnectionStringProvidermethod follows the established fluent builder pattern, has proper documentation, and correctly uses the generic type parameters for type-safe provider registration.src/Testcontainers/Configurations/Containers/ContainerConfiguration.cs (3)
65-91: LGTM!The new
connectionStringProviderparameter and its assignment follow the established pattern for other configuration properties in this class.
140-140: LGTM!Using
BuildConfiguration.Combinefor the connection string provider in the copy/merge constructor is consistent with how other configuration properties are handled.
223-226: LGTM!The
[JsonIgnore]attribute is appropriate since the provider contains runtime logic and references that shouldn't be serialized. This is consistent with other similar properties likeStartupCallbackandOutputConsumer.docs/api/connection_string_provider.md (1)
1-78: LGTM!The documentation is comprehensive, with clear examples for both generic and module-specific providers. The note about modules not yet implementing the feature sets appropriate expectations for users.
src/Testcontainers/Containers/DockerContainer.cs (3)
32-32: LGTM!The new private field follows the naming convention of other private fields in this class.
421-441: LGTM!Both
GetConnectionStringoverloads correctly check for null provider and throwConnectionStringProviderNotConfiguredExceptionwith a clear message. The delegation to the underlying provider is straightforward.
587-593: LGTM!The provider wiring is correctly placed after the container has started and readiness checks have passed. This ensures that
Configurereceives a fully initialized container with valid state, allowing the provider to access container information like ports and hostname.src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (1)
22-25: The casts rely on the framework ensuring type consistency.The casts to
TContainerEntityandTConfigurationEntitywill throwInvalidCastExceptionif the container/configuration types don't match. Since this is an internal class and the framework controls instantiation viaContainerBuilder.WithConnectionStringProvider, this is acceptable. The type constraints and builder pattern ensure correct usage.src/Testcontainers/Configurations/Containers/IConnectionStringProvider`2.cs (3)
1-4: LGTM!Namespace and using statements are appropriate.
6-10: Contravariance correctly applied.The
inmodifiers on both generic type parameters are appropriate since they're used only as input to theConfiguremethod. The generic constraints ensure type safety while the contravariance enables flexibility in the adapter pattern.
12-17: LGTM!The
Configuremethod signature is clean and enables the provider to access both container and configuration information. The design integrates well with the adapter pattern shown inConnectionStringProvider.cs.
src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (1)
16-18: Consider adding null validation for the constructor parameter.Although this is an internal class, adding a null check would improve defensive programming and make debugging easier if the class is misused.
Apply this diff to add validation:
public ConnectionStringProvider(IConnectionStringProvider<TContainerEntity, TConfigurationEntity> connectionStringProvider) { + ArgumentNullException.ThrowIfNull(connectionStringProvider); _connectionStringProvider = connectionStringProvider; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/api/connection_string_provider.md(1 hunks)src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (1)
src/Testcontainers/Containers/DockerContainer.cs (2)
GetConnectionString(422-430)GetConnectionString(433-441)
🪛 LanguageTool
docs/api/connection_string_provider.md
[style] ~4-~4: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1503 characters long)
Context: ... API base addresses) in a uniform way. !!!note Testcontainers modules do not ...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
docs/api/connection_string_provider.md
13-13: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (59)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Cassandra, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ClickHouse, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Azurite, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ArangoDb, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.Bigtable, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.ActiveMq, ubuntu-24.04)
- GitHub Check: ci (Testcontainers.BigQuery, ubuntu-24.04)
🔇 Additional comments (7)
src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs (3)
1-9: LGTM! Clean adapter pattern setup.The class structure correctly implements the adapter pattern by wrapping a generic
IConnectionStringProvider<TContainerEntity, TConfigurationEntity>and exposing it asIConnectionStringProvider<IContainer, IContainerConfiguration>. The generic constraints are appropriate.
12-14: Past review comment has been addressed.The XML documentation now correctly references
ConnectionStringProvider{TContainerEntity, TConfigurationEntity}instead ofContainerConfiguration.
27-37: LGTM! Correct delegation pattern.Both
GetConnectionStringoverloads correctly delegate to the wrapped provider with no additional logic, which is appropriate for an adapter.docs/api/connection_string_provider.md (4)
1-8: LGTM! Clear introduction and appropriate expectations.The documentation clearly explains the purpose of the Connection String Provider API and sets proper expectations that modules will integrate this in future releases. The
!!!notesyntax is correct MkDocs admonition markup, despite the static analysis hint.
9-26: LGTM! Clear usage examples.The example effectively demonstrates the three primary ways to retrieve connection strings: implicit host mode (default), explicit host mode, and container-to-container mode. The code is easy to follow.
28-55: LGTM! Comprehensive implementation guide.The generic provider example clearly demonstrates how to implement
IConnectionStringProvider<IContainer, IContainerConfiguration>with helpful inline comments explaining the purpose of each method and whenConfigureis invoked.
57-76: LGTM! Good forward-looking example.The module-specific provider example effectively demonstrates how future module integrations will work with typed containers and configurations. While
PostgreSqlContainerandPostgreSqlConfigurationdon't exist yet, this example helps developers understand the intended pattern, and the note at the beginning of the document already explains that modules will integrate this in future releases.
src/Testcontainers/Configurations/Containers/ConnectionStringProvider.cs
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
docs/api/connection_string_provider.md (1)
13-26: Align code block style with project markdown conventions.The code block uses fenced syntax (
```csharp) which may not align with your project's markdown style guide. According to markdownlint, consider using indented code blocks for consistency if your project prefers that style.If your project uses indented code blocks, refactor as follows:
- ```csharp - IContainer container = new ContainerBuilder() - .WithConnectionStringProvider(new MyProvider1()) - .Build(); - - // Implicit host connection string (default) - var hostConnectionStringImplicit = container.GetConnectionString(); - - // Explicit host connection string - var hostConnectionStringExplicit = container.GetConnectionString(ConnectionMode.Host); - - // Container-to-container connection string - var containerConnectionString = container.GetConnectionString(ConnectionMode.Container); - ``` + IContainer container = new ContainerBuilder() + .WithConnectionStringProvider(new MyProvider1()) + .Build(); + + // Implicit host connection string (default) + var hostConnectionStringImplicit = container.GetConnectionString(); + + // Explicit host connection string + var hostConnectionStringExplicit = container.GetConnectionString(ConnectionMode.Host); + + // Container-to-container connection string + var containerConnectionString = container.GetConnectionString(ConnectionMode.Container);Alternatively, if fenced blocks are your standard, you can ignore this suggestion.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/api/connection_string_provider.md(1 hunks)tests/Testcontainers.Platform.Linux.Tests/ConnectionStringProviderTest.cs(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/Testcontainers.Platform.Linux.Tests/ConnectionStringProviderTest.cs
🧰 Additional context used
🪛 LanguageTool
docs/api/connection_string_provider.md
[style] ~4-~4: Using many exclamation marks might seem excessive (in this case: 3 exclamation marks for a text that’s 1345 characters long)
Context: ... API base addresses) in a uniform way. !!!note Testcontainers modules do not ...
(EN_EXCESSIVE_EXCLAMATION)
🪛 markdownlint-cli2 (0.18.1)
docs/api/connection_string_provider.md
13-13: Code block style
Expected: indented; Actual: fenced
(MD046, code-block-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci (Testcontainers, ubuntu-24.04)
- GitHub Check: analyze (csharp)
🔇 Additional comments (1)
docs/api/connection_string_provider.md (1)
1-81: Documentation is well-structured and comprehensive.The documentation effectively introduces the Connection String Provider API, clearly explains its purpose, and provides both generic and module-specific implementation examples. The note about modules not yet implementing the feature appropriately sets expectations. The examples are accurate and demonstrate the two-pronged approach (generic IContainer provider and module-specific providers).
What does this PR do?
This PR adds a connection string provider, which provides a standardized way to access and manage connection information for Testcontainers. Configuring connection strings for modules can sometimes be tricky or tedious, and this API aims to simplify that process.
This PR does not implement the connection string provider in modules yet. That will be done in the future. Modules would offer a default implementation that users can override or replace with their own. For now, I want to test whether the proposed solution effectively addresses the issue.
Usage
I'd appreciate feedback from the community.
Why is it important?
The connection string provider API lets developers configure a provider that resolves connection strings based on the container and its configuration.
Related issues
IConnectionStringProviderfor modules / container builder API #1074