diff --git a/Changelog.md b/Changelog.md index edea0c3..12d8c19 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,6 +10,8 @@ ### Bug Fixes +- Restore original type activation through decorators when a provider exposes ordinary availability probing and keyed resolution without a keyed availability probe. Require keyed probing only for non-null dependency lookup keys, while preserving native constructor rules, generic constraint checks, layer lifetimes and disposal ownership [#120](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/120). + - Honor ServiceKey injection only for non-null keyed contexts in DependsOn activation. Preserve ordinary registered services, null factory results and optional defaults for unkeyed/null-key parameters, while retaining named override priority and non-null key type validation [#112](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/112). - Preserve native constructor selection for decorated original type registrations, including ignored preferred-constructor attributes, parameter-type-set ambiguity, contextual keys, generic constraints and native-provider metadata probing, while retaining factory/map activation policies, layer lifetimes and disposal ownership [#111](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/111). diff --git a/docs/keyed-built-in-registration-probing.md b/docs/keyed-built-in-registration-probing.md index 964af11..1e11ff2 100644 --- a/docs/keyed-built-in-registration-probing.md +++ b/docs/keyed-built-in-registration-probing.md @@ -73,6 +73,10 @@ Mammoth providers supply their existing registration snapshot. For a recognized Custom probes continue to supply their public availability contract. Missing or incompatible metadata on the recognized native probe fails clearly with the existing recommendation to use Mammoth's provider factory. This reuse adds no new private member lookup; it broadens the descriptor fallback's use to native-compatible generic constraint validation. +[Issue #120](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/120) restores ordinary decoration for providers that expose `IServiceProviderIsService` and keyed resolution but omit `IServiceProviderIsKeyedService`. The original-type activator now requires the keyed availability probe only when planning a dependency with a non-null lookup key. Ordinary dependencies, null-key lookups and contextual `ServiceKey` injection do not require that probe. Decoration still uses keyed resolution for its private slots, and nonempty DependsOn maps retain their existing provider requirements. + +Ordinary generic-constraint checks use the ordinary probe as the metadata owner. Native DI exposes the same `CallSiteFactory` through both probe interfaces, so the same exact-type guard, descriptor field and weak cache preserve the #111 constraint checks without requesting the hidden keyed probe. Mammoth snapshots remain preferred, and custom probes keep their public availability contract. This changes no reflection lookup or metadata-shape guard. + ## Verification of the compatibility choice The initial implementation in [PR #115](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/pull/115), commit `3c6942bfea31dfb1dcd2e3446cb05808a3835810`, was verified as follows. Revalidate these contracts when changing the fallback or upgrading the DI dependency. diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.OrdinaryProbe.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.OrdinaryProbe.Tests.cs new file mode 100644 index 0000000..a74562c --- /dev/null +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.OrdinaryProbe.Tests.cs @@ -0,0 +1,264 @@ +using Mammoth.Extensions.DependencyInjection.Configuration; +using Microsoft.Extensions.DependencyInjection; + +namespace Mammoth.Extensions.DependencyInjection.Tests; + +[TestClass] +public class OrdinaryDecorationProbeRegressionTests +{ + public static IEnumerable OrdinaryCases() + { + foreach (var type in new[] { typeof(Parameterless), typeof(Ordinary), typeof(Generic), typeof(ContextKey) }) + foreach (var kind in new[] { "native", "snapshot", "diagnostics" }) + foreach (var lifetime in new[] { ServiceLifetime.Transient, ServiceLifetime.Scoped, ServiceLifetime.Singleton }) + foreach (var layers in new[] { 1, 2 }) + yield return [type, kind, lifetime, layers]; + } + + [TestMethod] + [DynamicData(nameof(OrdinaryCases))] + public void OrdinaryOriginalsNeedNoKeyedProbeAndRetainLifetimeAndDisposal( + Type type, string kind, ServiceLifetime lifetime, int layers) + { + // Native controls and decorated originals use identical ordinary, null-key, + // and non-null-key registrations. Only the factories see the probe wrapper. + foreach (var context in new[] { "ordinary", "null", "blue" }) + foreach (var ordinaryOnlyProbe in new[] { false, true }) + { + var counts = new Counts(); + var services = Services(counts); + Add(services, type, lifetime, context); + using var native = services.BuildServiceProvider(); + using var nativeScope = native.CreateScope(); + var expected = Resolve(nativeScope.ServiceProvider, context); + for (var layer = 0; layer < layers; layer++) services.Decorate(); + WrapFactories(services, counts, exposeKeyedProbe: false, ordinaryOnlyProbe); + using var provider = Build(services, kind); + var originals = new HashSet(); + var wrappers = new HashSet(); + IWork? first = null; + for (var scopeIndex = 0; scopeIndex < 2; scopeIndex++) + { + using (var scope = provider.CreateScope()) + for (var repeat = 0; repeat < 2; repeat++) + { + var result = Resolve(scope.ServiceProvider, context); + if (first == null) first = result; + else if (lifetime == ServiceLifetime.Singleton || (lifetime == ServiceLifetime.Scoped && scopeIndex == 0)) + Assert.AreSame(first, result); + else Assert.AreNotSame(first, result); + for (var layer = 0; layer < layers; layer++) + { + var wrapper = (Wrapper)result; + wrappers.Add(wrapper); + result = wrapper.Inner; + } + originals.Add(result); + Assert.AreEqual(type, result.GetType()); + Assert.AreEqual(expected.Selected, result.Selected); + Assert.AreEqual(expected.Key, result.Key); + } + foreach (var original in originals) + Assert.AreEqual(lifetime == ServiceLifetime.Singleton ? 0 : 1, original.Disposals); + } + var instances = lifetime == ServiceLifetime.Singleton ? 1 : lifetime == ServiceLifetime.Scoped ? 2 : 4; + Assert.HasCount(instances, originals); + Assert.HasCount(instances * layers, wrappers); + provider.Dispose(); + foreach (var original in originals) Assert.AreEqual(1, original.Disposals); + foreach (var wrapper in wrappers) Assert.AreEqual(1, wrapper.Disposals); + Assert.AreEqual(0, counts.KeyProbeRequests, "Ordinary planning and contextual key injection must not request a keyed probe."); + } + } + + [TestMethod] + [DataRow("native")] + [DataRow("snapshot")] + [DataRow("diagnostics")] + public void HiddenKeyedProbeKeepsNativeConstructorPoliciesAndGenericConstraintPlanning(string kind) + { + foreach (var type in new[] { typeof(Preferred), typeof(Subset), typeof(Ambiguous), typeof(InvalidGeneric) }) + { + var nativeCounts = new Counts(); + var counts = new Counts(); + var nativeServices = Configure(nativeCounts); + var services = Configure(counts); + using var native = nativeServices.BuildServiceProvider(); + services.Decorate(); + WrapFactories(services, counts, exposeKeyedProbe: false, ordinaryOnlyProbe: false); + using var provider = Build(services, kind); + using var scope = provider.CreateScope(); + if (type == typeof(InvalidGeneric)) + { + Assert.ThrowsExactly(() => native.GetRequiredService()); + Assert.ThrowsExactly(() => scope.ServiceProvider.GetRequiredService()); + Assert.AreEqual(0, counts.Parts); + Assert.AreEqual(0, nativeCounts.Parts); + } + else if (type == typeof(Ambiguous)) + { + Assert.ThrowsExactly(() => native.GetRequiredService()); + var error = Assert.ThrowsExactly(() => scope.ServiceProvider.GetRequiredService()); + StringAssert.Contains(error.Message, "ambiguous"); + Assert.AreEqual(0, counts.Parts); + Assert.AreEqual(0, nativeCounts.Parts); + } + else + { + var expected = native.GetRequiredService(); + var result = scope.ServiceProvider.GetRequiredService(); + Assert.AreEqual(expected.Selected, result.Selected); + Assert.AreEqual(nativeCounts.Parts, counts.Parts); + } + Assert.AreEqual(0, counts.KeyProbeRequests); + + IServiceCollection Configure(Counts tracker) + { + var collection = Services(tracker); + collection.AddSingleton(); + if (type == typeof(InvalidGeneric)) + collection.AddTransient(typeof(IOpen<>), typeof(ReferenceOnly<>)); + Add(collection, type, ServiceLifetime.Transient, "ordinary"); + return collection; + } + } + } + + [TestMethod] + [DataRow("native", false)] + [DataRow("native", true)] + [DataRow("snapshot", false)] + [DataRow("snapshot", true)] + [DataRow("diagnostics", false)] + [DataRow("diagnostics", true)] + public void ActualKeyedDependenciesStillRequireTheirAvailabilityProbe(string kind, bool exposeKeyedProbe) + { + foreach (var type in new[] { typeof(Explicit), typeof(Inherited) }) + { + var counts = new Counts(); + var services = Services(counts); + services.AddKeyedSingleton("fixed", new Part()); + services.AddKeyedSingleton("blue", new Part()); + Add(services, type, ServiceLifetime.Transient, "blue"); + services.Decorate(); + services.Decorate(); + WrapFactories(services, counts, exposeKeyedProbe, ordinaryOnlyProbe: false); + using var provider = Build(services, kind); + using var scope = provider.CreateScope(); + if (exposeKeyedProbe) + Assert.AreEqual(type == typeof(Explicit) ? "explicit" : "inherited", Resolve(scope.ServiceProvider, "blue").Selected); + else + { + var error = Assert.ThrowsExactly(() => Resolve(scope.ServiceProvider, "blue")); + StringAssert.Contains(error.Message, nameof(IServiceProviderIsKeyedService)); + Assert.AreEqual(0, counts.Parts); + } + Assert.IsGreaterThan(0, counts.KeyProbeRequests); + } + } + + private static IServiceCollection Services(Counts counts) + { + IServiceCollection services = new ServiceCollection(); + services.AddTransient(_ => { counts.Parts++; return new Part(); }); + services.AddTransient(typeof(IOpen<>), typeof(Open<>)); + return services; + } + private static void Add(IServiceCollection services, Type type, ServiceLifetime lifetime, string context) + => services.Add(context == "ordinary" ? ServiceDescriptor.Describe(typeof(IWork), type, lifetime) + : ServiceDescriptor.DescribeKeyed(typeof(IWork), context == "null" ? null : context, type, lifetime)); + private static IWork Resolve(IServiceProvider provider, string context) => context == "ordinary" + ? provider.GetRequiredService() : provider.GetRequiredKeyedService(context == "null" ? null : context); + private static ServiceProvider Build(IServiceCollection services, string kind) => kind switch + { + "snapshot" => ServiceProviderFactory.CreateServiceProvider(services), + "diagnostics" => ServiceProviderFactory.CreateServiceProvider(services, + new ExtendedServiceProviderOptions { ValidateOnBuild = true, DetectIncorrectUsageOfTransientDisposables = true }), + _ => services.BuildServiceProvider() + }; + private static void WrapFactories(IServiceCollection services, Counts counts, bool exposeKeyedProbe, bool ordinaryOnlyProbe) + { + // Model a provider passing its own wrapper to factories. Keyed resolution is + // retained for Mammoth's private decoration slots; availability is independent. + for (var index = 0; index < services.Count; index++) + { + var descriptor = services[index]; + if (descriptor.IsKeyedService && descriptor.KeyedImplementationFactory is { } keyedFactory) + services[index] = ServiceDescriptor.DescribeKeyed(descriptor.ServiceType, descriptor.ServiceKey, + (provider, key) => keyedFactory(new ProbeProvider(provider, counts, exposeKeyedProbe, ordinaryOnlyProbe), key), descriptor.Lifetime); + else if (!descriptor.IsKeyedService && descriptor.ImplementationFactory is { } factory) + services[index] = ServiceDescriptor.Describe(descriptor.ServiceType, + provider => factory(new ProbeProvider(provider, counts, exposeKeyedProbe, ordinaryOnlyProbe)), descriptor.Lifetime); + } + } + public sealed class Counts { public int Parts; public int KeyProbeRequests; } + public sealed class ProbeProvider(IServiceProvider provider, Counts counts, bool exposeKeyedProbe, bool ordinaryOnlyProbe) + : IServiceProvider, IKeyedServiceProvider + { + public object? GetService(Type type) + { + if (type == typeof(IServiceProviderIsKeyedService)) + { + counts.KeyProbeRequests++; + return exposeKeyedProbe ? provider.GetService(type) : null; + } + if (ordinaryOnlyProbe && type == typeof(IServiceProviderIsService)) + return new OrdinaryProbe(provider.GetRequiredService()); + return provider.GetService(type); + } + public object? GetKeyedService(Type type, object? key) => ((IKeyedServiceProvider)provider).GetKeyedService(type, key); + public object GetRequiredKeyedService(Type type, object? key) => ((IKeyedServiceProvider)provider).GetRequiredKeyedService(type, key); + } + public sealed class OrdinaryProbe(IServiceProviderIsService probe) : IServiceProviderIsService + { + public bool IsService(Type type) => probe.IsService(type); + } + public interface IWork { string Selected { get; } object? Key { get; } int Disposals { get; } } + public abstract class Work(string selected, object? key = null) : IWork, IDisposable + { + public string Selected { get; } = selected; + public object? Key { get; } = key; + public int Disposals { get; private set; } + public void Dispose() => Disposals++; + } + public sealed class Parameterless() : Work("parameterless"); + public sealed class Ordinary(Part part) : Work("ordinary") { public Part Part { get; } = part; } + public sealed class Generic(IOpen part) : Work("generic") { public IOpen Part { get; } = part; } + public sealed class ContextKey([ServiceKey] object? key = null) : Work("context", key); + public sealed class Explicit([FromKeyedServices("fixed")] Part part) : Work("explicit") { public Part Part { get; } = part; } + public sealed class Inherited([FromKeyedServices] Part part) : Work("inherited") { public Part Part { get; } = part; } + public sealed class Preferred : Work + { + [ActivatorUtilitiesConstructor] public Preferred() : base("short") { } + public Preferred(Part part) : base("long") { } + } + public sealed class Subset : Work + { + public Subset(Part part) : base("short") { } + public Subset(Part part, Other other) : base("long") { } + } + public sealed class Ambiguous : Work + { + public Ambiguous(Part part) : base("part") { } + public Ambiguous(Other other) : base("other") { } + } + public sealed class InvalidGeneric : Work + { + public InvalidGeneric(Part part) : base("valid") { } + public InvalidGeneric(IOpen invalid, Missing missing) : base("invalid") { } + } + public sealed class Part; + public sealed class Other; + public sealed class Missing; + public interface IOpen; + public sealed class Open : IOpen; + public sealed class ReferenceOnly : IOpen where T : class; + public sealed class Wrapper(IWork inner) : IWork, IDisposable + { + public IWork Inner { get; } = inner; + public string Selected => Inner.Selected; + public object? Key => Inner.Key; + public int Disposals { get; private set; } + public void Dispose() => Disposals++; + } +} diff --git a/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs b/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs index 860fa88..0bfb15d 100644 --- a/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs +++ b/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs @@ -47,12 +47,14 @@ internal static bool IsKeyedBuiltInRegistered(IServiceProvider provider, IServic || registrations.Contains(new ServiceIdentifier(KeyedService.AnyKey, type)); } - private static readonly ConditionalWeakTable NativeSnapshots = new(); + private static readonly ConditionalWeakTable NativeSnapshots = new(); - internal static ServiceProviderRegistrationSnapshot? GetRegistrationSnapshot(IServiceProvider provider, IServiceProviderIsKeyedService probe) + internal static ServiceProviderRegistrationSnapshot? GetRegistrationSnapshot(IServiceProvider provider, IServiceProviderIsService probe) { if (provider.GetService() is { } snapshot) return snapshot; if (probe.GetType() != NativeProbeType) return null; + // The native ordinary and keyed probes expose the same descriptor owner. + // Ordinary constraint checks must not require a keyed probe registration. // Public probes cannot validate open-generic constraints on unselected candidates. // Reuse the guarded copied-descriptor fallback for original type activation, so // native providers need no Mammoth setup and no factory runs during selection. @@ -61,7 +63,7 @@ internal static bool IsKeyedBuiltInRegistered(IServiceProvider provider, IServic ReadNativeDescriptors(nativeProbe), [], [], [])); } - private static ServiceDescriptor[] ReadNativeDescriptors(IServiceProviderIsKeyedService nativeProbe) + private static ServiceDescriptor[] ReadNativeDescriptors(IServiceProviderIsService nativeProbe) { if (NativeDescriptors?.FieldType != typeof(ServiceDescriptor[]) || NativeDescriptors.GetValue(nativeProbe) is not ServiceDescriptor[] descriptors) diff --git a/src/Mammoth.Extensions.DependencyInjection/NativeConstructorActivator.cs b/src/Mammoth.Extensions.DependencyInjection/NativeConstructorActivator.cs index 8253fa5..3f6a546 100644 --- a/src/Mammoth.Extensions.DependencyInjection/NativeConstructorActivator.cs +++ b/src/Mammoth.Extensions.DependencyInjection/NativeConstructorActivator.cs @@ -11,7 +11,9 @@ internal static class NativeConstructorActivator internal static object CreateInstance(IServiceProvider provider, Type implementationType, object? serviceKey = null) { var probe = provider.GetRequiredService(); - var keyedProbe = provider.GetRequiredService(); + // Ordinary original activation needs only the ordinary probe. Acquire the + // keyed probe when a non-null dependency key actually requires planning it. + IServiceProviderIsKeyedService? keyedProbe = null; var constructors = implementationType.GetConstructors(); // Use the same arity ordering as native DI, including its tie ordering. Array.Sort(constructors, (left, right) => right.GetParameters().Length.CompareTo(left.GetParameters().Length)); @@ -106,9 +108,15 @@ internal static object CreateInstance(IServiceProvider provider, Type implementa bool IsRegistered(Type type, object? key) { + if (key == null) + { + if (type.IsConstructedGenericType) + ConstructorActivator.GetRegistrationSnapshot(provider, probe)?.ValidateGenericConstraints(type, key); + return probe.IsService(type); + } + keyedProbe ??= provider.GetRequiredService(); if (type.IsConstructedGenericType) ConstructorActivator.GetRegistrationSnapshot(provider, keyedProbe)?.ValidateGenericConstraints(type, key); - if (key == null) return probe.IsService(type); // Native's public keyed probe reports built-ins for every key, although // their implicit call sites are unkeyed. Explicit keyed registrations work. if (type == typeof(IServiceProvider) || type == typeof(IServiceScopeFactory)