From 64efacd78c1b05ac64c3fa9d3149cd8c69dbc48a Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Tue, 4 Aug 2026 10:46:16 +0200 Subject: [PATCH 1/2] fix(flags): allow the documented tuple config for flags and entitlements IFeatureFlags and IEntitlements were constrained to `where TConfig : class`, while their own typeparam docs promised "the configuration type (or value tuple of types)" and guide/flags/defining-flags.md documents "Multiple Config Sources" with `IFeatureFlags<(FeatureConfig, TenantConfig)>`. A tuple is a ValueTuple and therefore a struct, so the documented pattern never compiled: error CS0452: The type '(FeatureConfig, TenantConfig)' must be a reference type in order to use it as parameter 'TConfig' Dropping the constraint is the whole fix. The generator maps TConfig straight onto IReactiveConfig, which is itself unconstrained, so tuples work end to end once the interfaces stop rejecting them. Adds tests covering a flag class and an entitlement class over a named tuple, both reading from either element. Also converts the preferPendingState added in #54 to : documenting a single parameter made the compiler warn about the five undocumented ones (CS1573). The two remaining CS1574 cref warnings predate this branch. Co-Authored-By: Claude Opus 5 (1M context) --- .../Core/ConfigurationAccessor.cs | 15 ++-- .../Flags/IEntitlements.cs | 8 ++- .../Flags/IFeatureFlags.cs | 8 ++- .../TupleConfigFlagsTests.cs | 70 +++++++++++++++++++ 4 files changed, 90 insertions(+), 11 deletions(-) create mode 100644 src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs diff --git a/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs b/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs index 3a798bd..4f35d57 100644 --- a/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs +++ b/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs @@ -28,13 +28,14 @@ internal partial class ConfigurationAccessor : IConfigurationAccessor private readonly bool _preferPendingState; private ConfigManagerCapabilityScope? _capabilityScope; - /// - /// Resolve from the configuration state before the backplane. The backplane only ever holds the last - /// COMMITTED snapshot, so a rule factory reading it mid-recompute sees the previous pass — which is what - /// made config-aware rules (derived paths, URLs, values) lag or freeze. The engine's accessor sets this so - /// factories observe the in-flight pass, exactly as guide/configuration/config-aware.md describes; accessors - /// handed to application code leave it false and keep reading committed state only. - /// + /// + /// resolves from the configuration state before the backplane. The + /// backplane only ever holds the last COMMITTED snapshot, so a rule factory reading it mid-recompute sees the + /// previous pass — which is what made config-aware rules (derived paths, URLs, values) lag or freeze. The + /// engine's accessor sets it so factories observe the in-flight pass, exactly as + /// guide/configuration/config-aware.md describes; accessors handed to application code leave it false and + /// keep reading committed state only. + /// public ConfigurationAccessor( ConfigurationState state, ExposureRegistry bindingRegistry, diff --git a/src/Cocoar.Configuration/Flags/IEntitlements.cs b/src/Cocoar.Configuration/Flags/IEntitlements.cs index 4ea0221..669f1cf 100644 --- a/src/Cocoar.Configuration/Flags/IEntitlements.cs +++ b/src/Cocoar.Configuration/Flags/IEntitlements.cs @@ -5,7 +5,11 @@ namespace Cocoar.Configuration.Flags; /// Implement this on a partial class — the source generator produces the constructor and Config property. /// Entitlements are permanent and have no expiration. /// -/// The configuration type (or value tuple of types) this entitlement class reads from. -public interface IEntitlements where TConfig : class +/// +/// The configuration type this entitlement class reads from, or a value tuple of types to read from several at +/// once. Unconstrained on purpose: the generator maps this straight onto IReactiveConfig<TConfig>, +/// which is equally unconstrained, and a class constraint would reject the documented tuple form. +/// +public interface IEntitlements { } diff --git a/src/Cocoar.Configuration/Flags/IFeatureFlags.cs b/src/Cocoar.Configuration/Flags/IFeatureFlags.cs index 74180f9..f1b9bd4 100644 --- a/src/Cocoar.Configuration/Flags/IFeatureFlags.cs +++ b/src/Cocoar.Configuration/Flags/IFeatureFlags.cs @@ -5,8 +5,12 @@ namespace Cocoar.Configuration.Flags; /// Implement this on a partial class — the source generator produces the constructor, Config property, /// and IsExpired property. /// -/// The configuration type (or value tuple of types) this flag class reads from. -public interface IFeatureFlags where TConfig : class +/// +/// The configuration type this flag class reads from, or a value tuple of types to read from several at once. +/// Unconstrained on purpose: the generator maps this straight onto IReactiveConfig<TConfig>, which is +/// equally unconstrained, and a class constraint would reject the documented tuple form. +/// +public interface IFeatureFlags { /// /// When should these flags be removed from code? diff --git a/src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs b/src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs new file mode 100644 index 0000000..e026b9d --- /dev/null +++ b/src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs @@ -0,0 +1,70 @@ +using Cocoar.Configuration.Reactive; +using NSubstitute; + +namespace Cocoar.Configuration.Flags.Tests; + +public class FeatureConfig +{ + public bool NewCheckoutEnabled { get; set; } +} + +public class TenantConfig +{ + public bool AllowExperiments { get; set; } +} + +/// +/// The "Multiple Config Sources" pattern from guide/flags/defining-flags.md, using a named value tuple. The +/// interface's own typeparam doc promises "the configuration type (or value tuple of types)". +/// +public partial class RolloutFlags : IFeatureFlags<(FeatureConfig Features, TenantConfig Tenant)> +{ + public DateTimeOffset ExpiresAt => new(2099, 9, 1, 0, 0, 0, TimeSpan.Zero); + + public bool NewCheckout() => Config.Features.NewCheckoutEnabled && Config.Tenant.AllowExperiments; +} + +/// Same for entitlements, which carry the identical typeparam promise. +public partial class TenantEntitlements : IEntitlements<(FeatureConfig Features, TenantConfig Tenant)> +{ + public bool MayExperiment() => Config.Tenant.AllowExperiments; +} + +public class TupleConfigFlagsTests +{ + [Fact] + public void FlagClass_WithTupleConfig_ReadsBothConfigs() + { + var reactive = Substitute.For>(); + reactive.CurrentValue.Returns(( + new FeatureConfig { NewCheckoutEnabled = true }, + new TenantConfig { AllowExperiments = true })); + + var flags = new RolloutFlags(reactive); + + Assert.True(flags.NewCheckout()); + Assert.Equal(new DateTimeOffset(2099, 9, 1, 0, 0, 0, TimeSpan.Zero), flags.ExpiresAt); + } + + [Fact] + public void FlagClass_WithTupleConfig_RequiresBothConfigs() + { + var reactive = Substitute.For>(); + reactive.CurrentValue.Returns(( + new FeatureConfig { NewCheckoutEnabled = true }, + new TenantConfig { AllowExperiments = false })); + + Assert.False(new RolloutFlags(reactive).NewCheckout()); + } + + [Fact] + public void EntitlementClass_WithTupleConfig_ReadsItsConfig() + { + var reactive = Substitute.For>(); + reactive.CurrentValue.Returns(( + new FeatureConfig(), + new TenantConfig { AllowExperiments = true })); + + Assert.True(new TenantEntitlements(reactive).MayExperiment()); + } +} From 7d10d890c9eb4c39f87d9c6e00952026142eb08a Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Tue, 4 Aug 2026 11:22:08 +0200 Subject: [PATCH 2/2] fix(di): register the reactive config a tuple-based flag class needs Dropping the class constraint made the documented tuple pattern compile, but only the non-DI path actually worked. A flag or entitlement class is registered as its own implementation type, so the container builds it and must resolve the IReactiveConfig its generated constructor asks for. Per-config-type reactive services are emitted from the rule plan, so IReactiveConfig<(FeatureConfig, TenantConfig)> was never registered: System.InvalidOperationException: Unable to resolve service for type 'IReactiveConfig`1[ValueTuple`2[DiFeatureConfig,DiTenantConfig]]' while attempting to activate 'DiRolloutFlags'. That is very likely why the constraint existed: it turned an unsupported combination into a compile error instead of an obscure container failure. Removing it without this would have traded CS0452 for a runtime crash. EmitFlagsServices/EmitEntitlementsServices now derive TConfig from the class's IFeatureFlags/IEntitlements interface and emit the reactive registration when it is not already present. EmitReactiveService was already reflection-based and works for tuples unchanged. Adds container-level tests for both a flag class and an entitlement class over a tuple config; the earlier tests constructed the classes directly and never exercised this path. Co-Authored-By: Claude Opus 5 (1M context) --- .../ServiceDescriptorEmitter.cs | 33 ++++++++ .../TupleConfigFlagsDiTests.cs | 76 +++++++++++++++++++ .../TupleConfigFlagsTests.cs | 39 ++++++++++ 3 files changed, 148 insertions(+) create mode 100644 src/tests/Cocoar.Configuration.DI.Tests/TupleConfigFlagsDiTests.cs diff --git a/src/Cocoar.Configuration.DI/ServiceDescriptorEmitter.cs b/src/Cocoar.Configuration.DI/ServiceDescriptorEmitter.cs index 8991460..4b81c5d 100644 --- a/src/Cocoar.Configuration.DI/ServiceDescriptorEmitter.cs +++ b/src/Cocoar.Configuration.DI/ServiceDescriptorEmitter.cs @@ -111,7 +111,10 @@ private static void EmitFlagsServices(IServiceCollection services, ConfigManager services.AddSingleton(capability.Descriptors); foreach (var r in capability.Registrations) + { services.Add(new ServiceDescriptor(r.Descriptor.Type, r.Descriptor.Type, ServiceLifetime.Singleton)); + EmitReactiveServiceForClassConfig(services, r.Descriptor.Type, typeof(IFeatureFlags<>)); + } // Register all resolver types with lifetimes from Capabilities EmitResolverServices(services, @@ -132,7 +135,10 @@ private static void EmitEntitlementsServices(IServiceCollection services, Config services.AddSingleton(capability.Descriptors); foreach (var r in capability.Registrations) + { services.Add(new ServiceDescriptor(r.Descriptor.Type, r.Descriptor.Type, ServiceLifetime.Singleton)); + EmitReactiveServiceForClassConfig(services, r.Descriptor.Type, typeof(IEntitlements<>)); + } // Register all resolver types with lifetimes from Capabilities EmitResolverServices(services, @@ -216,6 +222,33 @@ private static void EmitConfigService( } } + /// + /// A flag or entitlement class is registered as its own implementation type, so the container builds it and + /// must resolve the its generated constructor asks for. When the class reads + /// from a value tuple of configs — the documented "Multiple Config Sources" pattern — that service type is not + /// one of the per-config-type registrations emitted from the rule plan, so it has to be added here. + /// + private static void EmitReactiveServiceForClassConfig( + IServiceCollection services, Type flagClassType, Type openFlagInterface) + { + var configType = flagClassType.GetInterfaces() + .FirstOrDefault(i => i.IsGenericType && i.GetGenericTypeDefinition() == openFlagInterface) + ?.GetGenericArguments()[0]; + + if (configType is null) + { + return; + } + + var reactiveType = typeof(IReactiveConfig<>).MakeGenericType(configType); + if (services.Any(d => d.ServiceType == reactiveType)) + { + return; + } + + EmitReactiveService(services, configType); + } + private static void EmitReactiveService(IServiceCollection services, Type serviceType) { var reactiveType = typeof(IReactiveConfig<>).MakeGenericType(serviceType); diff --git a/src/tests/Cocoar.Configuration.DI.Tests/TupleConfigFlagsDiTests.cs b/src/tests/Cocoar.Configuration.DI.Tests/TupleConfigFlagsDiTests.cs new file mode 100644 index 0000000..36a7983 --- /dev/null +++ b/src/tests/Cocoar.Configuration.DI.Tests/TupleConfigFlagsDiTests.cs @@ -0,0 +1,76 @@ +using Xunit; +using Cocoar.Configuration.DI; +using Cocoar.Configuration.Flags; +using Cocoar.Configuration.Providers; +using Microsoft.Extensions.DependencyInjection; + +namespace Cocoar.Configuration.DI.Tests; + +public class DiFeatureConfig +{ + public bool NewCheckoutEnabled { get; set; } +} + +public class DiTenantConfig +{ + public bool AllowExperiments { get; set; } +} + +/// +/// The "Multiple Config Sources" pattern from guide/flags/defining-flags.md, resolved through the container. +/// +public partial class DiRolloutFlags : IFeatureFlags<(DiFeatureConfig Features, DiTenantConfig Tenant)> +{ + public DateTimeOffset ExpiresAt => new(2099, 9, 1, 0, 0, 0, TimeSpan.Zero); + + public bool NewCheckout() => Config.Features.NewCheckoutEnabled && Config.Tenant.AllowExperiments; +} + +public partial class DiTenantEntitlements : IEntitlements<(DiFeatureConfig Features, DiTenantConfig Tenant)> +{ + public bool MayExperiment() => Config.Tenant.AllowExperiments; +} + +/// +/// A flag class is registered as its own implementation type, so the container constructs it and has to resolve +/// the IReactiveConfig<TConfig> its generated constructor asks for. For a tuple config that is a +/// different service type than the per-config-type reactive registrations, which is the case this covers. +/// +public class TupleConfigFlagsDiTests +{ + [Fact] + public void TupleConfigFlagClass_ResolvesFromTheContainer() + { + var services = new ServiceCollection(); + services.AddCocoarConfiguration(c => c + .UseConfiguration(rules => + [ + rules.For().FromStaticJson("""{"NewCheckoutEnabled":true}"""), + rules.For().FromStaticJson("""{"AllowExperiments":true}"""), + ]) + .UseFeatureFlags(flags => [flags.Register()])); + + using var sp = services.BuildServiceProvider(); + + var flags = sp.GetRequiredService(); + + Assert.True(flags.NewCheckout()); + } + + [Fact] + public void TupleConfigEntitlementClass_ResolvesFromTheContainer() + { + var services = new ServiceCollection(); + services.AddCocoarConfiguration(c => c + .UseConfiguration(rules => + [ + rules.For().FromStaticJson("""{"NewCheckoutEnabled":false}"""), + rules.For().FromStaticJson("""{"AllowExperiments":true}"""), + ]) + .UseEntitlements(e => [e.Register()])); + + using var sp = services.BuildServiceProvider(); + + Assert.True(sp.GetRequiredService().MayExperiment()); + } +} diff --git a/src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs b/src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs index e026b9d..8ce2310 100644 --- a/src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs +++ b/src/tests/Cocoar.Configuration.Flags.Tests/TupleConfigFlagsTests.cs @@ -1,3 +1,5 @@ +using Cocoar.Configuration.Core; +using Cocoar.Configuration.Providers; using Cocoar.Configuration.Reactive; using NSubstitute; @@ -57,6 +59,43 @@ public void FlagClass_WithTupleConfig_RequiresBothConfigs() Assert.False(new RolloutFlags(reactive).NewCheckout()); } + /// + /// The path that actually matters: registration via UseFeatureFlags and resolution through + /// GetFeatureFlags<T>(), which reflects over the generated constructor and calls + /// ConfigManager.GetReactiveConfig<TConfig>() with the tuple type. Constructing the class + /// directly (as the tests above do) would not exercise that. + /// + [Fact] + public void FlagClass_WithTupleConfig_ResolvesThroughTheConfigManager() + { + using var manager = ConfigManager.Create(c => c + .UseConfiguration(rules => + [ + rules.For().FromStaticJson("""{"NewCheckoutEnabled":true}"""), + rules.For().FromStaticJson("""{"AllowExperiments":true}"""), + ]) + .UseFeatureFlags(flags => [flags.Register()])); + + var flags = manager.GetFeatureFlags(); + + Assert.True(flags.NewCheckout()); + } + + /// Same for the entitlement path, which resolves through its own setup and cache. + [Fact] + public void EntitlementClass_WithTupleConfig_ResolvesThroughTheConfigManager() + { + using var manager = ConfigManager.Create(c => c + .UseConfiguration(rules => + [ + rules.For().FromStaticJson("""{"NewCheckoutEnabled":false}"""), + rules.For().FromStaticJson("""{"AllowExperiments":true}"""), + ]) + .UseEntitlements(e => [e.Register()])); + + Assert.True(manager.GetEntitlements().MayExperiment()); + } + [Fact] public void EntitlementClass_WithTupleConfig_ReadsItsConfig() {