feat: IConfigureOptions with IConfiguration binding for existing options classes - #677
Merged
samtrion merged 4 commits intoAug 3, 2026
Conversation
Add IConfigureOptions<TOptions> implementations that bind LoggingInterceptorOptions, TimeoutRequestInterceptorOptions, QueryCachingOptions, OutboxOptions, OutboxProcessorOptions, AzureServiceBusTransportOptions, RabbitMqTransportOptions, and DaprMessageTransportOptions from documented Pulse:* configuration sections, registered inside the respective existing Add*/Use* extension methods so IConfiguration-backed values are validated at startup by the #238 validators. AddRequestTimeout only applies its explicit globalTimeout parameter when a value is provided, so a configuration-bound GlobalTimeout is no longer unconditionally overwritten by the method's default null argument.
Contributor
|
Important Review skippedAuto reviews are limited based on label configuration. 🏷️ Required labels (at least one) (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
4 tasks
…238-options-validation
Resolving IRabbitMqChannelPool now requires IConfiguration to be resolvable, since RabbitMqTransportOptionsConfiguration (added by the IConfigureOptions binding work) depends on it.
samtrion
added a commit
that referenced
this pull request
Aug 3, 2026
…lasses (#676) * feat: add IValidateOptions and ValidateOnStart for existing options classes Add IValidateOptions<T> validators for TimeoutRequestInterceptorOptions, QueryCachingOptions, OutboxOptions, OutboxProcessorOptions, AzureServiceBusTransportOptions, RabbitMqTransportOptions, and DaprMessageTransportOptions, each registered with AddOptions<T>().ValidateOnStart() so misconfiguration is caught at startup instead of at first use. The AzureServiceBus transport's imperative ValidateOptions() check is replaced by AzureServiceBusTransportOptionsValidator with equivalent behavior, now surfaced as an OptionsValidationException instead of an InvalidOperationException. LoggingInterceptorOptions is intentionally left untouched: it already has a validator but is out of scope for this change. SQLiteOutboxOptions does not exist as a distinct type; the SQLite provider reuses the shared OutboxOptions, which is already covered by OutboxOptionsValidator (TableName not empty). ConnectionString remains unvalidated there since it is legitimately null for EF Core-based outbox usage. * fix(outbox): reword test comment to avoid false-positive S125 match SonarAnalyzer flagged the explanatory comment as commented-out code because it contained a code-like fragment; reworded in prose only. * feat: IConfigureOptions with IConfiguration binding for existing options classes (#677) * feat: bind options from configuration via IConfigureOptions Add IConfigureOptions<TOptions> implementations that bind LoggingInterceptorOptions, TimeoutRequestInterceptorOptions, QueryCachingOptions, OutboxOptions, OutboxProcessorOptions, AzureServiceBusTransportOptions, RabbitMqTransportOptions, and DaprMessageTransportOptions from documented Pulse:* configuration sections, registered inside the respective existing Add*/Use* extension methods so IConfiguration-backed values are validated at startup by the #238 validators. AddRequestTimeout only applies its explicit globalTimeout parameter when a value is provided, so a configuration-bound GlobalTimeout is no longer unconditionally overwritten by the method's default null argument. * fix(rabbitmq): register IConfiguration in channel pool resolution test Resolving IRabbitMqChannelPool now requires IConfiguration to be resolvable, since RabbitMqTransportOptionsConfiguration (added by the IConfigureOptions binding work) depends on it.
samtrion
added a commit
that referenced
this pull request
Aug 3, 2026
) * feat(rabbitmq): pool RabbitMQ channels in RabbitMqMessageTransport Replace the single lazily-created, publish-serialized channel in RabbitMqMessageTransport with a pooled IRabbitMqChannelPool / RabbitMqChannelPool backed by a ConcurrentQueue of idle channels and a SemaphoreSlim capped at the new RabbitMqTransportOptions.MaxChannelPoolSize (default 10). SendAsync rents a channel per call; SendBatchAsync rents a single channel for the whole batch and publishes sequentially on it, since only one thread ever touches that channel. Both always return the channel in a finally block. IsHealthyAsync now delegates to the pool. The channel pool is registered as a singleton via TryAddSingleton in UseRabbitMqTransport so repeated calls do not duplicate it. * fix(rabbitmq): suppress false-positive S5034 on per-iteration ValueTask conversion RentAsync_ConcurrentCalls_AreCappedAtMaxChannelPoolSize converts a fresh ValueTask returned by RentAsync to a Task exactly once per loop iteration, but SonarAnalyzer's cross-iteration analysis cannot tell the instances apart and flags a false double-consumption. * test(rabbitmq): cover channel-creation failure and pool resolution paths Adds coverage for RentAsync releasing its rental slot when channel creation fails, and for resolving IRabbitMqChannelPool from a built service provider, raising patch coverage to the required threshold. * feat: add IValidateOptions and ValidateOnStart for existing options classes (#676) * feat: add IValidateOptions and ValidateOnStart for existing options classes Add IValidateOptions<T> validators for TimeoutRequestInterceptorOptions, QueryCachingOptions, OutboxOptions, OutboxProcessorOptions, AzureServiceBusTransportOptions, RabbitMqTransportOptions, and DaprMessageTransportOptions, each registered with AddOptions<T>().ValidateOnStart() so misconfiguration is caught at startup instead of at first use. The AzureServiceBus transport's imperative ValidateOptions() check is replaced by AzureServiceBusTransportOptionsValidator with equivalent behavior, now surfaced as an OptionsValidationException instead of an InvalidOperationException. LoggingInterceptorOptions is intentionally left untouched: it already has a validator but is out of scope for this change. SQLiteOutboxOptions does not exist as a distinct type; the SQLite provider reuses the shared OutboxOptions, which is already covered by OutboxOptionsValidator (TableName not empty). ConnectionString remains unvalidated there since it is legitimately null for EF Core-based outbox usage. * fix(outbox): reword test comment to avoid false-positive S125 match SonarAnalyzer flagged the explanatory comment as commented-out code because it contained a code-like fragment; reworded in prose only. * feat: IConfigureOptions with IConfiguration binding for existing options classes (#677) * feat: bind options from configuration via IConfigureOptions Add IConfigureOptions<TOptions> implementations that bind LoggingInterceptorOptions, TimeoutRequestInterceptorOptions, QueryCachingOptions, OutboxOptions, OutboxProcessorOptions, AzureServiceBusTransportOptions, RabbitMqTransportOptions, and DaprMessageTransportOptions from documented Pulse:* configuration sections, registered inside the respective existing Add*/Use* extension methods so IConfiguration-backed values are validated at startup by the #238 validators. AddRequestTimeout only applies its explicit globalTimeout parameter when a value is provided, so a configuration-bound GlobalTimeout is no longer unconditionally overwritten by the method's default null argument. * fix(rabbitmq): register IConfiguration in channel pool resolution test Resolving IRabbitMqChannelPool now requires IConfiguration to be resolvable, since RabbitMqTransportOptionsConfiguration (added by the IConfigureOptions binding work) depends on it. * fix(rabbitmq): register IConfiguration in channel pool resolution test Resolving IOptions<RabbitMqTransportOptions> now requires IConfiguration to be resolvable, since RabbitMqTransportOptionsConfiguration depends on it; the raw ServiceCollection built by this integration test did not register one.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
IConfigureOptions<TOptions>implementation for each existing options class, binding the documentedPulse:*configuration section (configuration.GetSection("Pulse:...").Bind(options)), and registers each viaservices.TryAddEnumerable(ServiceDescriptor.Singleton<IConfigureOptions<TOptions>, TOptionsConfiguration>())inside the existingAdd*/Use*extension method (no new public API):LoggingInterceptorOptions→Pulse:Logging(AddLogging)TimeoutRequestInterceptorOptions→Pulse:Timeout(AddRequestTimeout)QueryCachingOptions→Pulse:QueryCaching(AddQueryCaching)OutboxOptions→Pulse:Outbox(AddOutbox)OutboxProcessorOptions→Pulse:OutboxProcessor(AddOutbox)AzureServiceBusTransportOptions→Pulse:Transports:AzureServiceBus(UseAzureServiceBusTransport)RabbitMqTransportOptions→Pulse:Transports:RabbitMq(UseRabbitMqTransport)DaprMessageTransportOptions→Pulse:Transports:Dapr(UseDaprTransport)IConfigureOptionsruns beforeIValidateOptionsin the options pipeline, a badPulse:*config section now fails at startup via the feat: IValidateOptions and ValidateOnStart for all existing options classes #238 validators, once bound.Configure(...)call in each method, so an explicit code-based configuration action still takes precedence over configuration-bound values (config supplies the base, code can override it).AddRequestTimeout(TimeSpan? globalTimeout = null)now only applies itsglobalTimeoutparameter toGlobalTimeoutwhen a value is actually supplied, instead of unconditionally overwriting it with the defaultnullargument. Without this change, a configuration-boundGlobalTimeoutwould always be wiped out by the parameterlessAddRequestTimeout()call.Deviation from the issue text:
SQLiteOutboxOptionsThe issue lists a
SQLiteOutboxOptionstype bound fromPulse:Outbox:SQLite. As already established in #238, no such type exists — the SQLite provider reuses the sharedOutboxOptions(seesrc/NetEvolve.Pulse.SQLite/). This row is skipped rather than inventing a new options type or a duplicate configurator:OutboxOptionsis already bound fromPulse:Outboxby item 4 above, which covers the SQLite provider as well. Introducing a SQLite-specificPulse:Outbox:SQLiteoverride section would only add a second, harder-to-reason-about binding path onto the same options instance for no documented benefit, so it was intentionally left out.Dependency
Depends on #238 (options validators), which this branch is based on (
feature/238-options-validation). This PR's diff should only contain the #239-specific changes once #238 is merged first.Closes #239
Test plan
dotnet build Pulse.slnxsucceeds with no new warnings/errorsdotnet run --project Tests/NetEvolve.Pulse.Tests.Unit -f net10.0 -- --treenode-filter "/*/*/*ConfigurationTests*/*"— new configurator tests (binding + default-preservation + null-configuration guard) all passNetEvolve.Pulse.Tests.Unitsuite (1629 tests) passes, including existing extension-method tests updated to register a minimalIConfiguration(previously implicit, now required sinceIConfigurationmust be resolvable from DI for the new configurators)csharpier format .applied