diff --git a/ZB.MOM.WW.Secrets/Directory.Build.props b/ZB.MOM.WW.Secrets/Directory.Build.props index c3118d2..0829136 100644 --- a/ZB.MOM.WW.Secrets/Directory.Build.props +++ b/ZB.MOM.WW.Secrets/Directory.Build.props @@ -5,7 +5,7 @@ enable enable latest - 0.2.0 + 0.2.1 true README.md diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs index 97e7383..1fd0523 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet/DependencyInjection/AkkaSecretsServiceCollectionExtensions.cs @@ -66,9 +66,6 @@ public static class AkkaSecretsServiceCollectionExtensions section.Bind(eager); eager.Validate(); - // The local store + migrator come from AddZbSecrets (SQLite by default). - services.AddZbSecrets(config, secretsSectionPath); - // The actor is created lazily on first use rather than at registration: the ActorSystem is // typically registered by the application AFTER this call, and a cluster node may also not be // ready to join at the moment the container is built. @@ -78,9 +75,17 @@ public static class AkkaSecretsServiceCollectionExtensions sp.GetService(), sp.GetRequiredService>().Value)); + // MUST be registered BEFORE AddZbSecrets. That call does + // TryAddSingleton(), so if it ran first it would + // win and the TryAdd below would be silently discarded — leaving replication bound to the + // no-op sink, publishing nothing, with no actor ever created and no error anywhere. + // Shipped that way in 0.2.0; fixed in 0.2.1. The SqlServer package always had this order. services.TryAddSingleton(sp => new AkkaSecretReplicator(sp.GetRequiredService().ActorRef)); + // The local store + migrator come from AddZbSecrets (SQLite by default). + services.AddZbSecrets(config, secretsSectionPath); + // Decorate the local store so every write publishes. Resolved from the concrete SQLite store, // not ISecretStore — asking the container for ISecretStore here would resolve this very // decorator and recurse forever. diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/AddZbSecretsAkkaReplicationTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/AddZbSecretsAkkaReplicationTests.cs new file mode 100644 index 0000000..39151af --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/DependencyInjection/AddZbSecretsAkkaReplicationTests.cs @@ -0,0 +1,109 @@ +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using ZB.MOM.WW.Secrets.Abstractions; +using ZB.MOM.WW.Secrets.DependencyInjection; +using ZB.MOM.WW.Secrets.Replication; +using ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.DependencyInjection; +using ZB.MOM.WW.Secrets.Sqlite; + +namespace ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.DependencyInjection; + +/// +/// Registration tests for AddZbSecretsAkkaReplication. +/// +/// These exist because 0.2.0 shipped with Akka replication silently inert and every other test in +/// this assembly still passed. The bug was pure registration ORDER: the extension called +/// AddZbSecrets first, which does +/// TryAddSingleton<ISecretReplicator, NoOpSecretReplicator>(), so the package's own +/// TryAddSingleton<ISecretReplicator> found a descriptor already present and was +/// silently discarded. Writes then published into a no-op sink and no replication actor was ever +/// spawned — with no exception and no log line anywhere. +/// +/// +/// The equivalent SQL-Server package always had a test asserting its replicator type, and always +/// had the correct order. That asymmetry is exactly why the defect landed in one package and not +/// the other, so the gap is closed here. +/// +/// +/// These assert against the rather than a built provider on +/// purpose. Resolving ISecretReplicator eagerly spawns the replication actor, whose +/// PreStart calls DistributedPubSub.Get(...); that requires a joined cluster, so a +/// provider-based test would need a real single-node cluster and would hang without one. The +/// defect was entirely about which descriptor wins, and the descriptor is where it is visible. +/// End-to-end actor behaviour is already covered by TwoNodeClusterReplicationTests. +/// +/// +public sealed class AddZbSecretsAkkaReplicationTests +{ + private static IServiceCollection Register() + { + IConfigurationRoot config = new ConfigurationBuilder() + .AddInMemoryCollection(new Dictionary + { + ["Secrets:SqlitePath"] = Path.Combine(Path.GetTempPath(), $"zb-akkadi-{Guid.NewGuid():N}.db"), + ["Secrets:Replication:AnnounceInterval"] = "00:00:30", + }) + .Build(); + + var services = new ServiceCollection(); + services.AddLogging(); + services.AddZbSecretsAkkaReplication(config, "Secrets", "Secrets:Replication"); + return services; + } + + private static ServiceDescriptor Descriptor(IServiceCollection services) => + services.Single(d => d.ServiceType == typeof(T)); + + [Fact] + public void ISecretReplicator_is_not_bound_to_the_no_op_sink() + { + // THE REGRESSION TEST. Before the fix this descriptor was NoOpSecretReplicator, so every + // replicated write went nowhere. A no-op secret replicator is the worst failure mode + // available: the cluster reports healthy and silently never converges. + ServiceDescriptor descriptor = Descriptor(Register()); + + Assert.NotEqual(typeof(NoOpSecretReplicator), descriptor.ImplementationType); + } + + [Fact] + public void ISecretReplicator_is_registered_by_this_package_via_a_factory() + { + // The package registers through a factory (it needs the lazily-created actor ref), whereas + // the core no-op is registered by concrete type. A factory registration is therefore proof + // that this package's descriptor won the TryAdd race rather than core's. + ServiceDescriptor descriptor = Descriptor(Register()); + + Assert.NotNull(descriptor.ImplementationFactory); + Assert.Null(descriptor.ImplementationType); + } + + [Fact] + public void ISecretStore_is_decorated_so_writes_actually_publish() + { + // AddSingleton (not TryAdd) means the decorator is appended, and the LAST registration for + // a service type is what the container resolves. If this regressed to the bare SQLite + // store, writes would never publish. + ServiceDescriptor last = Register() + .Last(d => d.ServiceType == typeof(ISecretStore)); + + Assert.NotNull(last.ImplementationFactory); + } + + [Fact] + public void The_undecorated_concrete_store_is_registered() + { + // ReplicatingSecretStore resolves SqliteSecretStore by concrete type. Core registering it + // only behind ISecretStore was the OTHER defect this library shipped, so pin it here too. + IServiceCollection services = Register(); + + Assert.Contains(services, d => d.ServiceType == typeof(SqliteSecretStore)); + } + + [Fact] + public void The_store_migrator_seam_is_registered() + { + IServiceCollection services = Register(); + + Assert.Contains(services, d => d.ServiceType == typeof(ISecretsStoreMigrator)); + } +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.csproj b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.csproj index b51027a..d7118b4 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.csproj +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests/ZB.MOM.WW.Secrets.Replicator.AkkaDotNet.Tests.csproj @@ -12,6 +12,7 @@ +