From e98df8fd61fe5275796de784b48fb19d9fc473de Mon Sep 17 00:00:00 2001 From: Bernhard Windisch Date: Tue, 4 Aug 2026 09:51:47 +0200 Subject: [PATCH] fix(core): config-aware rules follow their source at runtime again A rule factory resolved GetConfig() against the committed backplane snapshot instead of the pass it was running in, so a derived value, file path, HTTP URL, environment prefix or .When() predicate kept deciding on the previous recompute. Where the changing part lives in the query options rather than the provider options, the rule never updated at all. Startup was unaffected because the backplane is not initialized yet and the accessor falls back to pending state, which is why this went unnoticed since 9723080 (v4.2.0) despite contradicting guide/configuration/config-aware.md. - ConfigurationAccessor takes preferPendingState; TenantPipeline exposes a dedicated RecomputeAccessor for the engine, while application-facing reads keep committed-only semantics - ConfigManager routes all five engine entry points through it; the runtime ScheduleRecompute was the load-bearing one, not the init paths - RuleManager folds the query key into the transform key so a changed file name, URL or prefix invalidates the rule cache - StaticJsonProviderOptions drops its GenerateProviderKey() => null override and uses the interface default, which already derives identity from the serialized options; StaticJsonProvider hands back a copy because a shared instance must not expose the buffer RuleManager zeroes No provider-specific behaviour: one central repair covers FromStatic, FromFile, FromHttp, FromEnvironment, .When() and chained derivations, and the chain reaches its leaf within a single pass. Every new test was run against the unfixed tree and fails there. Two adjacent findings are covered by skipped acceptance tests rather than fixed here: per-type reactive emission, and a replaced writable-store backend that never triggers a re-read. Co-Authored-By: Claude Opus 5 (1M context) --- .../Core/ConfigManager.cs | 15 +- .../Core/ConfigurationAccessor.cs | 37 ++- .../Core/TenantPipeline.cs | 26 +- .../StaticJsonProvider/StaticJsonProvider.cs | 4 +- .../StaticJsonProviderOptions.cs | 10 +- src/Cocoar.Configuration/Rules/RuleManager.cs | 7 +- .../StaticJsonProviderContractTests.cs | 30 ++ .../ConfigAwareProviderMatrixTests.cs | 132 +++++++++ .../ConfigAwareRollbackAndTenantTests.cs | 107 +++++++ .../ConfigAwareRuntimeStalenessTests.cs | 261 ++++++++++++++++++ .../Http/HttpConfigAwareUrlTests.cs | 65 +++++ .../ServiceBackedConfigAwareTests.cs | 119 ++++++++ 12 files changed, 789 insertions(+), 24 deletions(-) create mode 100644 src/tests/Cocoar.Configuration.Core.Tests/Providers/StaticJsonProviderContractTests.cs create mode 100644 src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareProviderMatrixTests.cs create mode 100644 src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRollbackAndTenantTests.cs create mode 100644 src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRuntimeStalenessTests.cs create mode 100644 src/tests/Cocoar.Configuration.Providers.Tests/Http/HttpConfigAwareUrlTests.cs create mode 100644 src/tests/Cocoar.Configuration.ServiceBacked.Tests/ServiceBackedConfigAwareTests.cs diff --git a/src/Cocoar.Configuration/Core/ConfigManager.cs b/src/Cocoar.Configuration/Core/ConfigManager.cs index d3641e3..b08e9d7 100644 --- a/src/Cocoar.Configuration/Core/ConfigManager.cs +++ b/src/Cocoar.Configuration/Core/ConfigManager.cs @@ -177,8 +177,7 @@ internal ConfigManager Initialize() composer?.Build(); _capabilityScope.Owner.GetComposition()?.UsingEach(c => c.Apply()); - // The global pipeline's recompute accessor is `this` — byte-identical to before. - _global.Initialize(this, ScheduleRecompute); + _global.Initialize(ScheduleRecompute); } return this; } @@ -194,7 +193,7 @@ internal async Task InitializeAsync(CancellationToken cancellatio composer?.Build(); _capabilityScope.Owner.GetComposition()?.UsingEach(c => c.Apply()); - await _global.InitializeAsync(this, ScheduleRecompute, cancellationToken).ConfigureAwait(false); + await _global.InitializeAsync(ScheduleRecompute, cancellationToken).ConfigureAwait(false); } return this; } @@ -340,7 +339,7 @@ public bool IsHealthy internal string HealthDescription => _state.HealthDescription; internal void ScheduleRecompute(int startIndex) => - _engine.ScheduleRecompute(_ruleManagers, this, startIndex); + _engine.ScheduleRecompute(_ruleManagers, _global.RecomputeAccessor, startIndex); internal Task? CurrentRecomputeTask => _engine.CurrentRecomputeTask; @@ -350,7 +349,7 @@ internal void ScheduleRecompute(int startIndex) => /// concurrent provider-change signal cannot cancel activation before Layer 2 has committed (ADR-006 §7 readiness). /// internal Task RecomputeNowAsync(int startIndex, CancellationToken cancellationToken = default) => - _engine.RecomputeAndUpdateHealthAsync(_ruleManagers, this, startIndex, cancellationToken); + _engine.RecomputeAndUpdateHealthAsync(_ruleManagers, _global.RecomputeAccessor, startIndex, cancellationToken); /// /// Runs the same direct recompute on every already-initialized tenant pipeline from . @@ -384,7 +383,7 @@ internal async Task RecomputeInitializedTenantsNowAsync(int startIndex, Cancella // isolates the narrow dispose-race (a tenant removed mid-fan-out can surface ObjectDisposed / // index races from its health update) so one removed/faulting tenant never blocks the others. await pipeline.Engine.RecomputeAndUpdateHealthAsync( - pipeline.RuleManagers, pipeline.Accessor, startIndex, cancellationToken).ConfigureAwait(false); + pipeline.RuleManagers, pipeline.RecomputeAccessor, startIndex, cancellationToken).ConfigureAwait(false); } catch { @@ -505,8 +504,8 @@ private async Task BuildTenantAsync(string tenantId, Cancellatio // lock-ordering hazards a shared seed-from-global path would carry. The trade-off is linear resource use // (N tenants re-run the base); the seed-from-global sharing optimization is a documented, deferred TODO. await pipeline.InitializeAsync( - pipeline.Accessor, - startIndex => pipeline.Engine.ScheduleRecompute(pipeline.RuleManagers, pipeline.Accessor, startIndex), + startIndex => pipeline.Engine.ScheduleRecompute( + pipeline.RuleManagers, pipeline.RecomputeAccessor, startIndex), cancellationToken).ConfigureAwait(false); return pipeline; diff --git a/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs b/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs index 30c1786..3a798bd 100644 --- a/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs +++ b/src/Cocoar.Configuration/Core/ConfigurationAccessor.cs @@ -25,19 +25,29 @@ internal partial class ConfigurationAccessor : IConfigurationAccessor private readonly ExposureRegistry _bindingRegistry; private readonly ILogger _logger; private readonly List _rules; + 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. + /// public ConfigurationAccessor( ConfigurationState state, ExposureRegistry bindingRegistry, ILogger logger, List rules, - string? tenant = null) + string? tenant = null, + bool preferPendingState = false) { _state = state; _bindingRegistry = bindingRegistry; _logger = logger; _rules = rules; + _preferPendingState = preferPendingState; Tenant = tenant; } @@ -59,6 +69,11 @@ internal void SetCapabilityScope(ConfigManagerCapabilityScope capabilityScope) /// No configuration rule is registered for type T. public T? GetConfig() where T : class { + if (_preferPendingState && HasConfigurationInState(typeof(T))) + { + return FallbackDeserialize(); + } + // First try the backplane (has cached instances after initialization) try { @@ -196,6 +211,11 @@ public bool TryGetConfig(out T? value) where T : class /// No configuration rule is registered for the type. public object GetConfig(Type type) { + if (_preferPendingState && HasConfigurationInState(type)) + { + return FallbackDeserialize(type); + } + // First try the backplane try { @@ -214,6 +234,21 @@ public object GetConfig(Type type) return FallbackDeserialize(type); } + /// + /// Whether the configuration state can serve this type right now. A type whose rules have not run yet in the + /// current pass has no entry, so the caller falls back to the committed backplane instead of throwing. + /// + private bool HasConfigurationInState(Type type) + { + var targetType = type; + if (type.IsInterface && _bindingRegistry.TryGetConcreteType(type, out var concreteType)) + { + targetType = concreteType; + } + + return _state.TryGetConfiguration(targetType, out var json) && json != null; + } + private object FallbackDeserialize(Type type) { // Try to resolve interface to concrete type diff --git a/src/Cocoar.Configuration/Core/TenantPipeline.cs b/src/Cocoar.Configuration/Core/TenantPipeline.cs index 48d9700..57c4095 100644 --- a/src/Cocoar.Configuration/Core/TenantPipeline.cs +++ b/src/Cocoar.Configuration/Core/TenantPipeline.cs @@ -38,6 +38,12 @@ internal sealed class TenantPipeline : IWritableStoreHost, IDisposable, IAsyncDi internal ConfigurationState State { get; } internal ProviderRegistry ProviderRegistry { get; } internal ConfigurationAccessor Accessor { get; } + + /// + /// The accessor handed to the engine for rule factories and .When() predicates. Resolves in-flight + /// pass state before the committed backplane; never handed to application code. + /// + internal ConfigurationAccessor RecomputeAccessor { get; } internal ReactiveConfigManager ReactiveConfigManager { get; } internal ReactiveConfigurationFactory ReactiveFactory { get; } internal ConfigurationEngine Engine { get; } @@ -73,6 +79,13 @@ internal TenantPipeline( ProviderRegistry = new ProviderRegistry(_logger, enableDiagnostics: false, factory: providerFactory); Accessor = new ConfigurationAccessor(State, _bindingRegistry, _logger, Rules, tenantId); Accessor.SetCapabilityScope(_capabilityScope); + + // Rule factories must observe the pass they are running in, not the last committed one — see + // guide/configuration/config-aware.md ("Re-evaluation on Change"). That is why the engine gets its own + // accessor: application-facing reads keep seeing committed state only. + RecomputeAccessor = new ConfigurationAccessor( + State, _bindingRegistry, _logger, Rules, tenantId, preferPendingState: true); + RecomputeAccessor.SetCapabilityScope(_capabilityScope); ReactiveConfigManager = new ReactiveConfigManager(_logger, _bindingRegistry); // Reactive reads bind to the global ConfigManager for the global pipeline (byte-identical) and to this @@ -84,24 +97,19 @@ internal TenantPipeline( Engine = new ConfigurationEngine(State, _logger); } - /// - /// The handed to the engine for recompute-window fallback reads and - /// provider option factories. For the global pipeline this is the owning - /// (byte-identical to before); for a tenant pipeline it is the tenant's own accessor. - /// - internal void Initialize(IConfigurationAccessor recomputeAccessor, Action scheduleRecompute) + internal void Initialize(Action scheduleRecompute) { Engine.InitializeAndCompute( - Rules, RuleManagers, ProviderRegistry, recomputeAccessor, + Rules, RuleManagers, ProviderRegistry, RecomputeAccessor, _bindingRegistry, _capabilityScope, scheduleRecompute, _debounceMilliseconds); ReactiveConfigManager.SetBackplane(State.Backplane); Volatile.Write(ref _initialized, 1); } - internal async Task InitializeAsync(IConfigurationAccessor recomputeAccessor, Action scheduleRecompute, CancellationToken cancellationToken) + internal async Task InitializeAsync(Action scheduleRecompute, CancellationToken cancellationToken) { await Engine.InitializeAndComputeAsync( - Rules, RuleManagers, ProviderRegistry, recomputeAccessor, + Rules, RuleManagers, ProviderRegistry, RecomputeAccessor, _bindingRegistry, _capabilityScope, scheduleRecompute, _debounceMilliseconds, cancellationToken).ConfigureAwait(false); ReactiveConfigManager.SetBackplane(State.Backplane); Volatile.Write(ref _initialized, 1); diff --git a/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProvider.cs b/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProvider.cs index 20cb7a6..61c44be 100644 --- a/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProvider.cs +++ b/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProvider.cs @@ -18,7 +18,9 @@ private static byte[] SerializeToBytes(JsonElement value) public override Task FetchConfigurationBytesAsync(StaticJsonProviderQueryOptions query, CancellationToken ct = default) { - return Task.FromResult(_cachedBytes); + // Callers may zero the array they receive, and one provider instance is shared by every + // rule with identical payload, so the cached buffer must never leave this object. + return Task.FromResult((byte[])_cachedBytes.Clone()); } public override IObservable ChangesAsBytes(StaticJsonProviderQueryOptions queryOptions) diff --git a/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProviderOptions.cs b/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProviderOptions.cs index ad63a9b..a1a383a 100644 --- a/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProviderOptions.cs +++ b/src/Cocoar.Configuration/Providers/StaticJsonProvider/StaticJsonProviderOptions.cs @@ -6,7 +6,9 @@ namespace Cocoar.Configuration.Providers; public record StaticJsonProviderQueryOptions() : IProviderQuery; -public record StaticJsonProviderOptions(JsonElement Value) : IProviderConfiguration -{ - public string? GenerateProviderKey() => null; -} +/// +/// Uses the default provider key (the serialized options), so the payload itself identifies the provider. A +/// config-aware FromStatic factory is re-evaluated every recompute, and a changed payload has to produce a +/// changed key — otherwise the lease reuses the first provider and the rule serves its startup value forever. +/// +public record StaticJsonProviderOptions(JsonElement Value) : IProviderConfiguration; diff --git a/src/Cocoar.Configuration/Rules/RuleManager.cs b/src/Cocoar.Configuration/Rules/RuleManager.cs index d224223..02d0cf5 100644 --- a/src/Cocoar.Configuration/Rules/RuleManager.cs +++ b/src/Cocoar.Configuration/Rules/RuleManager.cs @@ -81,7 +81,12 @@ public RuleManager(ConfigRule rule, ILogger logger, ProviderRegistry registry) var queryOptions = _rule.ResolveQueryOptions(accessor); EnsureSubscription(queryOptions); - var newTransformKey = ComputeTransformKey(_rule.Options); + + // The query says WHAT is read — file name, URL, environment prefix — and a config-aware factory may + // return a different one on the next pass. It is not part of the provider key (a file provider is keyed + // by directory), so without folding it in here the cached bytes of the PREVIOUS query would stay valid + // and the rule would keep serving the old source forever. + var newTransformKey = ComputeTransformKey(_rule.Options) + "|" + ComputeQueryKey(queryOptions); _cache.UpdateTransformKey(newTransformKey); try diff --git a/src/tests/Cocoar.Configuration.Core.Tests/Providers/StaticJsonProviderContractTests.cs b/src/tests/Cocoar.Configuration.Core.Tests/Providers/StaticJsonProviderContractTests.cs new file mode 100644 index 0000000..46accf0 --- /dev/null +++ b/src/tests/Cocoar.Configuration.Core.Tests/Providers/StaticJsonProviderContractTests.cs @@ -0,0 +1,30 @@ +using System.Text.Json; +using Cocoar.Configuration.Providers; + +namespace Cocoar.Configuration.Core.Tests.Providers; + +[Trait("Category", "Unit")] +[Trait("Component", "StaticJsonProvider")] +public class StaticJsonProviderContractTests +{ + /// + /// ConfigurationProvider requires a provider to "return fresh arrays a caller may zero", and + /// RuleManager.ComputeAsync does zero every buffer it receives. Handing out the provider's own cached + /// array therefore leaves the next fetch reading a run of zero bytes. + /// + [Fact] + [Trait("Type", "Unit")] + public async Task FetchDoesNotHandOutTheProvidersOwnBuffer() + { + using var document = JsonDocument.Parse("""{"Region":"eu"}"""); + var provider = new StaticJsonProvider(new StaticJsonProviderOptions(document.RootElement.Clone())); + var query = new StaticJsonProviderQueryOptions(); + + var first = await provider.FetchConfigurationBytesAsync(query); + Array.Clear(first, 0, first.Length); + + var second = await provider.FetchConfigurationBytesAsync(query); + + Assert.Equal("""{"Region":"eu"}""", System.Text.Encoding.UTF8.GetString(second)); + } +} diff --git a/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareProviderMatrixTests.cs b/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareProviderMatrixTests.cs new file mode 100644 index 0000000..0bac403 --- /dev/null +++ b/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareProviderMatrixTests.cs @@ -0,0 +1,132 @@ +using System.Reactive.Subjects; +using Cocoar.Configuration.Core.Tests.TestUtilities; +using Cocoar.Configuration.Fluent; +using Cocoar.Configuration.Providers; + +namespace Cocoar.Configuration.Core.Tests.Verification; + +/// +/// The library's promise is that the provider behind a rule does not change the semantics — every provider +/// yields JSON and the engine does the rest. These tests run the same config-aware scenario (an upstream value +/// changes at runtime, a dependent rule must follow) across the providers reachable from this project, so a +/// regression in one provider cannot hide behind another's coverage. +/// The HTTP case lives in Cocoar.Configuration.Providers.Tests, which is the project that references it. +/// +[Trait("Category", "Integration")] +[Trait("Component", "ConfigAware")] +public class ConfigAwareProviderMatrixTests +{ + public class SourceCfg + { + public string Name { get; set; } = "unset"; + public bool Enabled { get; set; } + } + + public class TargetCfg { public string Value { get; set; } = "unset"; } + + public class MiddleCfg { public string Value { get; set; } = "unset"; } + + public class LeafCfg { public string Value { get; set; } = "unset"; } + + [Fact] + [Trait("Type", "Unit")] + public async Task Environment_DerivedPrefix_FollowsItsSource() + { + var run = "MTX" + Guid.NewGuid().ToString("N")[..8].ToUpperInvariant(); + Environment.SetEnvironmentVariable($"{run}A_Value", "A"); + Environment.SetEnvironmentVariable($"{run}B_Value", "B"); + try + { + using var source = new BehaviorSubject($$"""{"Name":"{{run}}A_"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromEnvironment(a => + new EnvironmentVariableRuleOptions(a.GetConfig()!.Name)), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + Assert.Equal("A", mgr.GetConfig()!.Value); + + source.OnNext($$"""{"Name":"{{run}}B_"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Value == "B", + description: "environment rule to follow the derived prefix"); + + Assert.Equal("B", mgr.GetConfig()!.Value); + } + finally + { + Environment.SetEnvironmentVariable($"{run}A_Value", null); + Environment.SetEnvironmentVariable($"{run}B_Value", null); + } + } + + /// + /// .When() reads the same accessor as a provider factory, so a predicate must also see the pass it + /// runs in — otherwise a rule switches on or off one recompute late. + /// + [Fact] + [Trait("Type", "Unit")] + public async Task ConditionalRule_TogglesInTheSamePassAsItsSource() + { + using var source = new BehaviorSubject("""{"Enabled":false}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromStaticJson("""{"Value":"base"}"""), + builder.For().FromStaticJson("""{"Value":"overlay"}""") + .When(a => a.GetConfig()!.Enabled), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + Assert.Equal("base", mgr.GetConfig()!.Value); + + source.OnNext("""{"Enabled":true}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Value == "overlay", + description: "conditional rule to switch on"); + + source.OnNext("""{"Enabled":false}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Value == "base", + description: "conditional rule to switch off again"); + } + + /// + /// A → B → C in one rule list. Rules run in order within a pass, so a single upstream change has to reach + /// the leaf in that same pass rather than taking one recompute per link. + /// + [Fact] + [Trait("Type", "Unit")] + public async Task ChainedDerivations_ReachTheLeafInOnePass() + { + using var source = new BehaviorSubject("""{"Name":"one"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromStatic(a => + new MiddleCfg { Value = "mid-" + a.GetConfig()!.Name }), + builder.For().FromStatic(a => + new LeafCfg { Value = "leaf-" + a.GetConfig()!.Value }), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + Assert.Equal("leaf-mid-one", mgr.GetConfig()!.Value); + + source.OnNext("""{"Name":"two"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Name == "two", + description: "source to update"); + await Task.Delay(300); + + Assert.Equal("mid-two", mgr.GetConfig()!.Value); + Assert.Equal("leaf-mid-two", mgr.GetConfig()!.Value); + } +} diff --git a/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRollbackAndTenantTests.cs b/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRollbackAndTenantTests.cs new file mode 100644 index 0000000..5692536 --- /dev/null +++ b/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRollbackAndTenantTests.cs @@ -0,0 +1,107 @@ +using System.Reactive.Subjects; +using Cocoar.Configuration.Core.Tests.TestUtilities; +using Cocoar.Configuration.Fluent; +using Cocoar.Configuration.Providers; + +namespace Cocoar.Configuration.Core.Tests.Verification; + +/// +/// Rule factories resolve the in-flight pass rather than the last committed snapshot. That raises two questions +/// these tests answer: whether a pass that rolls back can leave a derived rule holding the value it computed +/// from uncommitted state, and whether a tenant pipeline follows its base the same way the global one does. +/// +[Trait("Category", "Integration")] +[Trait("Component", "ConfigAware")] +public class ConfigAwareRollbackAndTenantTests +{ + public class SourceCfg { public string Name { get; set; } = "unset"; } + + public class DerivedCfg { public string Value { get; set; } = "unset"; } + + public class StrictCfg { public string Value { get; set; } = "unset"; } + + /// + /// A required rule fails AFTER a derived rule already computed from the in-flight values. The whole pass must + /// roll back: neither the source nor the derived value may advance, and the derived rule must not keep the + /// value it produced from the discarded state — the next healthy pass has to correct it. + /// + [Fact] + [Trait("Type", "Unit")] + public async Task FailedPass_DoesNotPublishValuesDerivedFromDiscardedState() + { + using var source = new BehaviorSubject("""{"Name":"one"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromStatic(a => + new DerivedCfg { Value = "d-" + a.GetConfig()!.Name }), + builder.For().FromStatic(a => + a.GetConfig()!.Name == "poison" + ? throw new InvalidOperationException("simulated required-rule failure") + : new StrictCfg { Value = "s-" + a.GetConfig()!.Name }) + .Required(), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + + Assert.Equal("one", mgr.GetConfig()!.Name); + Assert.Equal("d-one", mgr.GetConfig()!.Value); + Assert.Equal("s-one", mgr.GetConfig()!.Value); + + // This pass computes a derived value from in-flight state and then fails on the required rule. + source.OnNext("""{"Name":"poison"}"""); + await Task.Delay(500); + + Assert.Equal("one", mgr.GetConfig()!.Name); + Assert.Equal("d-one", mgr.GetConfig()!.Value); + Assert.Equal("s-one", mgr.GetConfig()!.Value); + + // The engine must not be poisoned by the discarded pass: a healthy change still lands completely. + source.OnNext("""{"Name":"two"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Value == "d-two", + description: "recovery pass to land"); + + Assert.Equal("two", mgr.GetConfig()!.Name); + Assert.Equal("d-two", mgr.GetConfig()!.Value); + Assert.Equal("s-two", mgr.GetConfig()!.Value); + } + + /// + /// A tenant pipeline runs the same flat rule list against its own state, so a tenant-scoped derived rule has + /// to follow a change in the shared base exactly like the global pipeline does. + /// + [Fact] + [Trait("Type", "Unit")] + public async Task TenantScopedDerivedRule_FollowsTheSharedBase() + { + using var source = new BehaviorSubject("""{"Name":"one"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromStatic(a => + new DerivedCfg { Value = $"{a.Tenant}-{a.GetConfig()!.Name}" }).TenantScoped(), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + await mgr.InitializeTenantAsync("acme"); + await mgr.InitializeTenantAsync("globex"); + + Assert.Equal("acme-one", mgr.GetConfigForTenant("acme")!.Value); + Assert.Equal("globex-one", mgr.GetConfigForTenant("globex")!.Value); + + source.OnNext("""{"Name":"two"}"""); + + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfigForTenant("acme")!.Value == "acme-two", + timeout: TimeSpan.FromSeconds(5), + description: "tenant acme to follow the base change"); + + Assert.Equal("acme-two", mgr.GetConfigForTenant("acme")!.Value); + Assert.Equal("globex-two", mgr.GetConfigForTenant("globex")!.Value); + } +} diff --git a/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRuntimeStalenessTests.cs b/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRuntimeStalenessTests.cs new file mode 100644 index 0000000..01de1b4 --- /dev/null +++ b/src/tests/Cocoar.Configuration.Core.Tests/Verification/ConfigAwareRuntimeStalenessTests.cs @@ -0,0 +1,261 @@ +using System.Reactive.Subjects; +using Cocoar.Configuration.Core.Tests.TestUtilities; +using Cocoar.Configuration.Fluent; +using Cocoar.Configuration.Providers; + +namespace Cocoar.Configuration.Core.Tests.Verification; + +/// +/// Acceptance tests for behaviour the published documentation promises for config-aware rules +/// (guide/configuration/config-aware.md, "Re-evaluation on Change"). Between v4.2.0 and v6.1.0 a +/// derived value froze or lagged a recompute behind and a derived file path never followed its +/// source at all; the two rule-order tests below lock the repaired behaviour in. +/// +/// Background, measured traces and root-cause analysis: Atlas topic +/// Cocoar.Configuration / config-aware-rules-runtime-staleness. +/// +/// +/// The Probe* tests are diagnostics, not assertions — they end in Assert.Fail to print +/// a trace of what the engine actually did, so they stay skipped. +/// ChangingOneType_DoesNotEmitForAnUnchangedType covers a separate, still-open finding. +/// +/// +[Trait("Category", "Verification")] +public class ConfigAwareRuntimeStalenessTests +{ + public class SourceCfg + { + public string Region { get; set; } = "unset"; + public string Name { get; set; } = "unset"; + } + + public class DerivedCfg { public string Endpoint { get; set; } = "unset"; } + + public class TargetCfg { public string Value { get; set; } = "unset"; } + + public class OtherCfg { public string Value { get; set; } = "unset"; } + + // ---------------------------------------------------------------- acceptance + + /// + /// website/guide/configuration/config-aware.md, "Re-evaluation on Change": the factory is + /// re-evaluated per recompute and "reads the new region". + /// Observed: on a pristine 6c79b28 the derived value freezes; with a content-derived + /// StaticJson provider key it lags exactly one recompute behind. + /// + [Fact] + [Trait("Type", "Unit")] + public async Task DerivedStaticValue_FollowsItsSource() + { + using var source = new BehaviorSubject("""{"Region":"us"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromStatic(a => + new DerivedCfg { Endpoint = "db-" + a.GetConfig()!.Region }), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + Assert.Equal("db-us", mgr.GetConfig()!.Endpoint); + + source.OnNext("""{"Region":"eu"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Region == "eu", description: "source to become eu"); + await Task.Delay(300); + + Assert.Equal("db-eu", mgr.GetConfig()!.Endpoint); + } + + /// + /// Same doc page, "Dynamic file paths": a derived path must switch the rule to the new file. + /// Observed: never updates — the file name lives in the query options, which neither rebuild + /// the provider nor invalidate the rule's transform cache. + /// + [Fact] + [Trait("Type", "Unit")] + public async Task DerivedFilePath_FollowsItsSource() + { + var dir = CreateProbeDirectory(); + try + { + using var source = new BehaviorSubject("""{"Name":"a"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromFile(a => + FileSourceRuleOptions.FromFilePath( + Path.Combine(dir, a.GetConfig()!.Name + ".json"))), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + Assert.Equal("A", mgr.GetConfig()!.Value); + + source.OnNext("""{"Name":"b"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Name == "b", description: "source to become b"); + await Task.Delay(400); + + Assert.Equal("B", mgr.GetConfig()!.Value); + } + finally + { + TryDelete(dir); + } + } + + /// + /// website/guide/reactive/basics.md:76 — "If the data hasn't changed (same JSON content), the + /// same instance reference is reused and the subscriber is not called." + /// Observed: changing only type A also emits for type B and gives B a fresh instance, because + /// every commit re-deserializes every type. Impact is spurious wakeups, not wrong values. + /// + [Fact(Skip = "Known failing — documents the per-type emission gap. Remove Skip to reproduce.")] + [Trait("Type", "Unit")] + public async Task ChangingOneType_DoesNotEmitForAnUnchangedType() + { + using var subjA = new BehaviorSubject("""{"Value":"a1"}"""); + using var subjB = new BehaviorSubject("""{"Value":"b1"}"""); + + var rules = new List + { + TestRules.ObservableString(subjA), + TestRules.ObservableString(subjB), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + + var emissionsB = new List(); + using var sub = mgr.GetReactiveConfig().Subscribe(x => emissionsB.Add(x)); + + await ActiveWaitHelpers.WaitUntilAsync( + () => emissionsB.Count > 0, description: "initial emission for B"); + + var emissionsBefore = emissionsB.Count; + var refBefore = mgr.GetConfig(); + + subjA.OnNext("""{"Value":"a2"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Value == "a2", description: "A to update"); + await Task.Delay(300); + + Assert.True( + emissionsBefore == emissionsB.Count, + $"B emitted although only A changed: before={emissionsBefore}, after={emissionsB.Count}"); + Assert.Same(refBefore, mgr.GetConfig()); + } + + // ---------------------------------------------------------------- trace probes + + /// + /// Prints how a derived static value evolves across several source changes. Distinguishes + /// "frozen forever" from "lags exactly one recompute". + /// + [Fact(Skip = "Trace probe, not an assertion. Remove Skip to print the observed behaviour.")] + [Trait("Type", "Unit")] + public async Task ProbeDerivedStaticLag() + { + using var source = new BehaviorSubject("""{"Region":"us"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromStatic(a => + new DerivedCfg { Endpoint = "db-" + a.GetConfig()!.Region }), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + + var trace = new List + { + $"start: src={mgr.GetConfig()!.Region} derived={mgr.GetConfig()!.Endpoint}", + }; + + foreach (var region in new[] { "eu", "ap", "sa" }) + { + source.OnNext($$"""{"Region":"{{region}}"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Region == region, description: region); + await Task.Delay(300); + trace.Add($"after {region}: src={mgr.GetConfig()!.Region} derived={mgr.GetConfig()!.Endpoint}"); + } + + Assert.Fail("TRACE >>> " + string.Join(" | ", trace)); + } + + /// + /// Prints how a derived file path evolves across several source changes. + /// + [Fact(Skip = "Trace probe, not an assertion. Remove Skip to print the observed behaviour.")] + [Trait("Type", "Unit")] + public async Task ProbeDerivedFilePath() + { + var dir = CreateProbeDirectory(); + try + { + using var source = new BehaviorSubject("""{"Name":"a"}"""); + var builder = new RulesBuilder(); + + var rules = new List + { + TestRules.ObservableString(source), + builder.For().FromFile(a => + FileSourceRuleOptions.FromFilePath( + Path.Combine(dir, a.GetConfig()!.Name + ".json"))), + }; + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules).UseDebounce(50)); + + var trace = new List + { + $"start: src={mgr.GetConfig()!.Name} target={mgr.GetConfig()!.Value}", + }; + + foreach (var name in new[] { "b", "c" }) + { + source.OnNext($$"""{"Name":"{{name}}"}"""); + await ActiveWaitHelpers.WaitUntilAsync( + () => mgr.GetConfig()!.Name == name, description: name); + await Task.Delay(400); + trace.Add($"after {name}: src={mgr.GetConfig()!.Name} target={mgr.GetConfig()!.Value}"); + } + + Assert.Fail("TRACE >>> " + string.Join(" | ", trace)); + } + finally + { + TryDelete(dir); + } + } + + // ---------------------------------------------------------------- helpers + + private static string CreateProbeDirectory() + { + var dir = Path.Combine(Path.GetTempPath(), "cocoar-probe-" + Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + File.WriteAllText(Path.Combine(dir, "a.json"), """{"Value":"A"}"""); + File.WriteAllText(Path.Combine(dir, "b.json"), """{"Value":"B"}"""); + File.WriteAllText(Path.Combine(dir, "c.json"), """{"Value":"C"}"""); + return dir; + } + + private static void TryDelete(string dir) + { + try + { + Directory.Delete(dir, recursive: true); + } + catch (IOException) + { + // Probe artefact in the temp folder — losing it is harmless. + } + catch (UnauthorizedAccessException) + { + } + } +} diff --git a/src/tests/Cocoar.Configuration.Providers.Tests/Http/HttpConfigAwareUrlTests.cs b/src/tests/Cocoar.Configuration.Providers.Tests/Http/HttpConfigAwareUrlTests.cs new file mode 100644 index 0000000..6255ac6 --- /dev/null +++ b/src/tests/Cocoar.Configuration.Providers.Tests/Http/HttpConfigAwareUrlTests.cs @@ -0,0 +1,65 @@ +using System.Net; +using System.Reactive.Subjects; +using Cocoar.Configuration.Core; +using Cocoar.Configuration.Fluent; +using Cocoar.Configuration.Http; +using Xunit; + +namespace Cocoar.Configuration.Providers.Tests.Http; + +/// +/// The headline example of guide/configuration/config-aware.md: a poll URL derived from an upstream config. +/// When the upstream region changes the provider has to switch endpoints, in the same way a derived file path +/// switches files — the provider behind a rule must not change the semantics. +/// +[Trait("Category", "Integration")] +[Trait("Component", "ConfigAware")] +public class HttpConfigAwareUrlTests +{ + public class RegionCfg { public string Region { get; set; } = "unset"; } + + public class ApiCfg { public string Value { get; set; } = "unset"; } + + [Fact] + [Trait("Type", "Unit")] + public async Task DerivedUrl_SwitchesEndpointWhenItsSourceChanges() + { + var handler = new RegionRoutingHandler(); + using var source = new BehaviorSubject("""{"Region":"us"}"""); + + using var mgr = ConfigManager.Create(c => c.UseConfiguration(rules => + [ + rules.For().FromObservable(source), + rules.For().FromHttp(a => new( + url: $"https://example.com/{a.GetConfig()!.Region}/config", + pollInterval: TimeSpan.FromMilliseconds(50), + handler: handler)), + ]).UseDebounce(50)); + + Assert.Equal("US", mgr.GetConfig()!.Value); + + source.OnNext("""{"Region":"eu"}"""); + + var deadline = DateTime.UtcNow.AddSeconds(5); + while (DateTime.UtcNow < deadline && mgr.GetConfig()!.Value != "EU") + { + await Task.Delay(40); + } + + Assert.Equal("eu", mgr.GetConfig()!.Region); + Assert.Equal("EU", mgr.GetConfig()!.Value); + } + + private sealed class RegionRoutingHandler : HttpMessageHandler + { + protected override Task SendAsync( + HttpRequestMessage request, CancellationToken cancellationToken) + { + var region = request.RequestUri!.AbsolutePath.Contains("/eu/", StringComparison.Ordinal) ? "EU" : "US"; + return Task.FromResult(new HttpResponseMessage(HttpStatusCode.OK) + { + Content = new StringContent($$"""{"Value":"{{region}}"}"""), + }); + } + } +} diff --git a/src/tests/Cocoar.Configuration.ServiceBacked.Tests/ServiceBackedConfigAwareTests.cs b/src/tests/Cocoar.Configuration.ServiceBacked.Tests/ServiceBackedConfigAwareTests.cs new file mode 100644 index 0000000..a14a061 --- /dev/null +++ b/src/tests/Cocoar.Configuration.ServiceBacked.Tests/ServiceBackedConfigAwareTests.cs @@ -0,0 +1,119 @@ +using Cocoar.Configuration.Core; +using Cocoar.Configuration.DI; +using Cocoar.Configuration.Providers; +using Microsoft.Extensions.DependencyInjection; + +namespace Cocoar.Configuration.ServiceBacked.Tests; + +/// +/// A Layer-2 factory receives (IServiceProvider, IConfigurationAccessor), so it is config-aware in the +/// same sense as a Layer-1 factory: it must read the pass it runs in, both during the activation recompute and +/// on every later change to the Layer-1 value it depends on. +/// +[Trait("Category", "ServiceBacked")] +[Trait("Component", "ConfigAware")] +public class ServiceBackedConfigAwareTests +{ + public class SourceCfg { public string Name { get; set; } = "unset"; } + + public class RemoteCfg { public string Value { get; set; } = "unset"; } + + /// + /// Known failing, and deliberately narrower than it looks. Activation itself is correct — the factory reads + /// the Layer-1 value and the store is seeded from it. What does not work is a LATER Layer-1 change: the + /// factory runs again and returns a different backend, WritableStoreRulesExtensions detects that and + /// calls WritableStoreState.ReplaceBackend, but that method only swaps the field. Nothing signals a + /// change and nothing invalidates the rule's cache, so the store keeps serving what it read the first time. + /// Either the swap should force a re-read or it should not happen at all. Fixing it touches the writable-store + /// read/write model, so it is tracked separately rather than folded into the config-aware repair. + /// + [Fact(Skip = "Known failing: a replaced writable-store backend never triggers a re-read. Remove Skip to reproduce.")] + [Trait("Type", "Unit")] + public async Task Layer2Factory_ReadsLayer1_AtActivationAndOnLaterChanges() + { + var source = new ReplayingSource("""{"Name":"one"}"""); + + var services = new ServiceCollection(); + services.AddCocoarConfiguration(c => c + .UseConfiguration(rules => + [ + rules.For().FromObservable(source), + rules.For().FromStaticJson("""{ "Value": "base" }"""), + ]) + .UseServiceBackedConfiguration(rules => + [ + rules.For().FromStore((_, accessor) => + new SeededBackend($$"""{ "Value": "s-{{accessor.GetConfig()!.Name}}" }""")), + ]) + .UseDebounce(25)); + + await using var sp = services.BuildServiceProvider(); + var mgr = sp.GetRequiredService(); + + // Dormant until activation: Layer 1 wins. + Assert.Equal("base", mgr.GetConfig()!.Value); + + await sp.ActivateServiceBackedConfigurationAsync(); + Assert.Equal("s-one", mgr.GetConfig()!.Value); + + source.Push("""{"Name":"two"}"""); + + var deadline = DateTime.UtcNow.AddSeconds(5); + while (DateTime.UtcNow < deadline && mgr.GetConfig()!.Value != "s-two") + { + await Task.Delay(25); + } + + Assert.Equal("two", mgr.GetConfig()!.Name); + Assert.Equal("s-two", mgr.GetConfig()!.Value); + } + + /// + /// Minimal replay-1 source. This project deliberately does not reference System.Reactive, and the test only + /// needs "hand the current value to a new subscriber, then push updates". + /// + private sealed class ReplayingSource(string initial) : IObservable + { + private readonly List> _observers = []; + private string _current = initial; + + public IDisposable Subscribe(IObserver observer) + { + string current; + lock (_observers) + { + _observers.Add(observer); + current = _current; + } + + observer.OnNext(current); + return new Subscription(this, observer); + } + + public void Push(string value) + { + IObserver[] snapshot; + lock (_observers) + { + _current = value; + snapshot = [.. _observers]; + } + + foreach (var observer in snapshot) + { + observer.OnNext(value); + } + } + + private sealed class Subscription(ReplayingSource source, IObserver observer) : IDisposable + { + public void Dispose() + { + lock (source._observers) + { + source._observers.Remove(observer); + } + } + } + } +}