diff --git a/Changelog.md b/Changelog.md index 324c5fc..edea0c3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,6 +10,8 @@ ### Bug Fixes +- 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). - Preserve registered factory null results in mapped and contextual keyed constructor dependencies, including named key maps and decoration. Apply optional defaults only to missing registrations while retaining missing-required failures and original factory exceptions [#110](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/110). diff --git a/README.md b/README.md index efc1e20..ef8cd70 100644 --- a/README.md +++ b/README.md @@ -189,6 +189,8 @@ This extension supports Singleton, Scoped, Transient and Keyed registrations. Ma Non-empty maps choose a constructor at resolution time. A single `[ActivatorUtilitiesConstructor]` constructor must be satisfiable; otherwise the unique longest satisfiable public constructor wins. Equal-length ambiguity fails. Named overrides take precedence over explicit-key `[FromKeyedServices(key)]`, `[ServiceKey]` and ordinary injection; optional defaults apply only to unregistered dependencies, including null and non-null nullable-enum defaults. Rejected constructors do not create dependencies. Empty maps use native DI behavior. Custom providers need ordinary/keyed service probes and keyed resolution. Use explicit keys on the mapped attribute path rather than assuming newer native attribute lookup modes are supported. +`[ServiceKey]` injects only a non-null service key. For unkeyed or null-key registrations, the parameter uses its ordinary registered service or optional default. Named overrides still take precedence. + Constructor failures retain the original exception instance and stack trace; dependency-resolution failures pass through unchanged, including application-thrown `TargetInvocationException` instances. For example, use a key attribute and an optional default without registering the optional value: diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ConstructorActivator.UnkeyedServiceKey.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ConstructorActivator.UnkeyedServiceKey.Tests.cs new file mode 100644 index 0000000..d8af062 --- /dev/null +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ConstructorActivator.UnkeyedServiceKey.Tests.cs @@ -0,0 +1,225 @@ +using Mammoth.Extensions.DependencyInjection.Configuration; +using Microsoft.Extensions.DependencyInjection; + +namespace Mammoth.Extensions.DependencyInjection.Tests; + +[TestClass] +public class UnkeyedServiceKeyRegressionTests +{ + public static IEnumerable OrdinaryCases() + { + foreach (var kind in new[] { "native", "snapshot", "diagnostics" }) + foreach (var nullKey in new[] { false, true }) + foreach (var type in new[] { "string", "object", "int", "nullable" }) + foreach (var behavior in new[] { "registered", "missing", "null", "throw" }) + foreach (var lifetime in new[] { ServiceLifetime.Transient, ServiceLifetime.Scoped, ServiceLifetime.Singleton }) + if ((behavior == "registered" || lifetime == ServiceLifetime.Transient) && (behavior != "null" || type != "int")) + yield return [kind, nullKey, type, behavior, lifetime]; + } + + [TestMethod] + [DynamicData(nameof(OrdinaryCases))] + public void NullContextMatchesNativeRegistrationDefaultsAndFailures( + string kind, bool nullKey, string type, string behavior, ServiceLifetime lifetime) + { + var value = Value(type); + var error = new InvalidOperationException("Key dependency factory failure"); + foreach (var optional in new[] { false, true }) + { + var nativeCalls = new Calls(); + var calls = new Calls(); + using var native = Services(nativeCalls, false).BuildServiceProvider(); + using var provider = Build(Services(calls, true), kind); + Assert.AreEqual(0, calls.Keys); + Assert.AreEqual(0, calls.Markers); + for (var scopeIndex = 0; scopeIndex < 2; scopeIndex++) + { + using var nativeScope = native.CreateScope(); + using var scope = provider.CreateScope(); + for (var repeat = 0; repeat < 2; repeat++) + { + if (behavior == "throw" || (behavior == "missing" && !optional)) + { + var nativeError = Assert.ThrowsExactly(() => Resolve(nativeScope.ServiceProvider, nullKey, null)); + var actualError = Assert.ThrowsExactly(() => Resolve(scope.ServiceProvider, nullKey, null)); + if (behavior == "throw") + { + Assert.AreSame(error, nativeError); + Assert.AreSame(error, actualError); + } + else Assert.AreEqual(0, calls.Markers, "Missing required keys must reject selection before unrelated activation."); + } + else + { + var nativeResult = Resolve(nativeScope.ServiceProvider, nullKey, null); + var result = Resolve(scope.ServiceProvider, nullKey, null); + Assert.AreEqual(behavior == "missing" ? Default(type) : behavior == "null" ? null : value, nativeResult.Key); + Assert.AreEqual(nativeResult.Key, result.Key); + if (type == "object") Assert.AreSame(nativeResult.Key, result.Key); + Assert.IsTrue(result.Label); + } + Assert.AreEqual(nativeCalls.Keys, calls.Keys); + Assert.AreEqual(nativeCalls.Markers, calls.Markers); + } + } + + IServiceCollection Services(Calls tracker, bool mapped) + { + IServiceCollection services = new ServiceCollection(); + services.AddSingleton(typeof(bool), true); + services.AddTransient(_ => { tracker.Markers++; return new Marker(); }); + if (behavior != "missing") services.Add(ServiceDescriptor.Describe(ParameterType(type), _ => + { + tracker.Keys++; + if (behavior == "throw") throw error; + return behavior == "null" ? null! : value; + }, lifetime)); + Register(services, type, optional, mapped, nullKey, null, lifetime, [Dependency.OnValue("label", true)]); + return services; + } + } + } + + public static IEnumerable OverrideCases() + { + foreach (var kind in new[] { "native", "snapshot", "diagnostics" }) + foreach (var type in new[] { "string", "object", "int", "nullable" }) + foreach (var context in new[] { "ordinary", "null-key", "non-null" }) + foreach (var namedKey in new[] { false, true }) + yield return [kind, type, context, namedKey]; + } + + [TestMethod] + [DynamicData(nameof(OverrideCases))] + public void NamedOverridesWinBeforeServiceKeyAndOrdinaryDependencies( + string kind, string type, string context, bool namedKey) + { + IServiceCollection services = new ServiceCollection(); + var value = Value(type); + var rejectedCalls = 0; + services.AddSingleton(typeof(bool), true); + services.AddSingleton(); + services.Add(ServiceDescriptor.Describe(ParameterType(type), _ => { rejectedCalls++; throw new InvalidOperationException("Unselected ordinary dependency"); }, ServiceLifetime.Transient)); + if (namedKey) services.Add(ServiceDescriptor.KeyedSingleton(ParameterType(type), "override", value)); + Dependency[] map = [Dependency.OnValue("label", true), namedKey + ? Parameter.ForKey("key").Eq("override") : Dependency.OnValue("key", value)]; + // A string context intentionally mismatches value-type key parameters. The + // explicit named override must take precedence over contextual type checks. + var serviceKey = context == "non-null" ? "outer" : null; + var keyed = context != "ordinary"; + Register(services, type, false, true, keyed, serviceKey, ServiceLifetime.Transient, map); + using var provider = Build(services, kind); + var result = Resolve(provider, keyed, serviceKey); + Assert.AreEqual(value, result.Key); + if (type == "object") Assert.AreSame(value, result.Key); + Assert.AreEqual(0, rejectedCalls); + } + + public static IEnumerable KeyedCases() + { + foreach (var kind in new[] { "native", "snapshot", "diagnostics" }) + foreach (var type in new[] { "string", "object", "int" }) + foreach (var lifetime in new[] { ServiceLifetime.Transient, ServiceLifetime.Scoped, ServiceLifetime.Singleton }) + yield return [kind, type, lifetime]; + } + + [TestMethod] + [DynamicData(nameof(KeyedCases))] + public void NonNullContextStillInjectsItsActualTypedKeyWithoutOrdinaryActivation( + string kind, string type, ServiceLifetime lifetime) + { + var serviceKey = type == "int" ? (object)42 : "outer"; + var ordinaryCalls = 0; + IServiceCollection services = new ServiceCollection(); + services.AddSingleton(typeof(bool), true); + services.AddSingleton(); + services.Add(ServiceDescriptor.Describe(ParameterType(type), _ => { ordinaryCalls++; return Value(type); }, ServiceLifetime.Transient)); + Register(services, type, false, false, true, serviceKey, lifetime, []); + using var native = services.BuildServiceProvider(); + var nativeResult = Resolve(native, true, serviceKey); + services.Clear(); + services.AddSingleton(); + services.Add(ServiceDescriptor.Describe(ParameterType(type), _ => { ordinaryCalls++; return Value(type); }, ServiceLifetime.Transient)); + Register(services, type, false, true, true, serviceKey, lifetime, [Dependency.OnValue("label", true)]); + using var provider = Build(services, kind); + using var scope = provider.CreateScope(); + var result = Resolve(scope.ServiceProvider, true, serviceKey); + Assert.AreEqual(nativeResult.Key, result.Key); + if (type == "object") Assert.AreSame(serviceKey, result.Key); + Assert.AreEqual(0, ordinaryCalls); + } + + [TestMethod] + [DataRow("native")] + [DataRow("snapshot")] + [DataRow("diagnostics")] + public void NonNullMismatchedKeyTypeStillFailsBeforeActivation(string kind) + { + IServiceCollection services = new ServiceCollection(); + var calls = 0; + services.AddTransient(_ => { calls++; return new Marker(); }); + services.AddSingleton(typeof(int), 42); + Register(services, "int", false, true, true, "outer", ServiceLifetime.Transient, [Dependency.OnValue("label", true)]); + using var provider = Build(services, kind); + Assert.ThrowsExactly(() => Resolve(provider, true, "outer")); + Assert.AreEqual(0, calls); + } + + private static void Register(IServiceCollection services, string type, bool optional, bool mapped, + bool keyed, object? serviceKey, ServiceLifetime lifetime, Dependency[] map) + { + if (!optional) + { + if (type == "string") Add>(); + else if (type == "object") Add>(); + else if (type == "int") Add>(); + else Add>(); + } + else if (type == "string") Add(); + else if (type == "object") Add(); + else if (type == "int") Add(); + else Add(); + + void Add() where T : class, IValue + { + if (!mapped) services.Add(keyed ? ServiceDescriptor.DescribeKeyed(typeof(IValue), serviceKey, typeof(T), lifetime) + : ServiceDescriptor.Describe(typeof(IValue), typeof(T), lifetime)); + else if (keyed) + { + if (lifetime == ServiceLifetime.Singleton) services.AddKeyedSingleton(serviceKey, map); + else if (lifetime == ServiceLifetime.Scoped) services.AddKeyedScoped(serviceKey, map); + else services.AddKeyedTransient(serviceKey, map); + } + else if (lifetime == ServiceLifetime.Singleton) services.AddSingleton(map); + else if (lifetime == ServiceLifetime.Scoped) services.AddScoped(map); + else services.AddTransient(map); + } + } + + private static Type ParameterType(string type) => type switch { "string" => typeof(string), "object" => typeof(object), "int" => typeof(int), _ => typeof(int?) }; + private static object Value(string type) => type switch { "string" => "ordinary", "object" => new object(), _ => 42 }; + private static object? Default(string type) => type switch { "string" => "fallback", "int" => 77, "nullable" => 19, _ => null }; + private static IValue Resolve(IServiceProvider provider, bool keyed, object? key) => keyed + ? provider.GetRequiredKeyedService(key) : provider.GetRequiredService(); + 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() + }; + public sealed class Calls { public int Keys; public int Markers; } + public sealed class Marker; + public interface IValue { object? Key { get; } bool Label { get; } Marker Marker { get; } } + public abstract class Values(object? key, bool label, Marker marker) : IValue + { + public object? Key { get; } = key; + public bool Label { get; } = label; + public Marker Marker { get; } = marker; + } + public sealed class Required([ServiceKey] T key, Marker marker, bool label) : Values(key, label, marker); + public sealed class OptionalString(Marker marker, bool label, [ServiceKey] string key = "fallback") : Values(key, label, marker); + public sealed class OptionalObject(Marker marker, bool label, [ServiceKey] object? key = null) : Values(key, label, marker); + public sealed class OptionalInt(Marker marker, bool label, [ServiceKey] int key = 77) : Values(key, label, marker); + public sealed class OptionalNullable(Marker marker, bool label, [ServiceKey] int? key = 19) : Values(key, label, marker); +} diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs index fb9d6e4..7795110 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs @@ -170,7 +170,7 @@ public void KeyAwareConstructorSelectionPreservesDefaultsAndNativePreference(boo [TestMethod] [DataRow(false)] [DataRow(true)] - public void NullLookupModesPreserveUnkeyedDependsOnCompatibility(bool diagnostic) + public void NullLookupModesUseNativeUnkeyedServiceKeyResolution(bool diagnostic) { var services = Services(); services.AddKeyedTransient(null); @@ -179,7 +179,7 @@ public void NullLookupModesPreserveUnkeyedDependsOnCompatibility(bool diagnostic using var provider = Build(services, diagnostic); using var scope = provider.CreateScope(); var work = scope.ServiceProvider.GetRequiredService(); - Assert.IsNull(work.Key, "Unkeyed DependsOn retains its existing null ServiceKey injection contract."); + Assert.AreEqual("ordinary-key", work.Key, "Null-key DependsOn uses ordinary registration resolution, as native DI does."); Assert.AreEqual("ordinary", work.Inherited.Name); Assert.AreEqual("ordinary", work.Ordinary.Name); } diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Integration.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Integration.Tests.cs index bb9ed6b..1f9f83e 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Integration.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedService.Integration.Tests.cs @@ -15,6 +15,8 @@ public class DependsOnMergedFeatureIntegrationTests public void NamedGenericDependenciesIgnorePublicMutationAndKeepClosedPrecedence(bool diagnostics, bool keyed) { var services = new ServiceCollection(); + // With no effective key, ServiceKey resolves the ordinary object registration. + services.AddSingleton(new object()); services.AddTransient(typeof(IRepository<>), typeof(UnkeyedRepository<>)); services.AddKeyedScoped(typeof(IRepository<>), "blue", typeof(BlueRepository<>)); services.AddKeyedSingleton(typeof(IRepository<>), "red", typeof(RedRepository<>)); @@ -42,7 +44,7 @@ public void NamedGenericDependenciesIgnorePublicMutationAndKeepClosedPrecedence( Assert.IsInstanceOfType>(result.Left); Assert.IsInstanceOfType>(result.Right); Assert.IsInstanceOfType(result.Closed); - Assert.AreEqual(keyed ? "outer" : null, result.Key); + Assert.AreEqual(keyed ? "outer" : provider.GetRequiredService(), result.Key); Assert.AreEqual(7, result.Count); Assert.AreEqual(0, rejectedCreations); } diff --git a/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs b/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs index 98966df..860fa88 100644 --- a/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs +++ b/src/Mammoth.Extensions.DependencyInjection/ConstructorActivator.cs @@ -153,9 +153,12 @@ bool CanSupply(ParameterMetadata parameter) return dependency.T == Dependency.DependencyType.KeyedServices ? IsKeyedRegistered(parameter.ParameterType, dependency.Value) : AcceptsValue(parameter.ParameterType, dependency.Value); - if (parameter.IsServiceKey) + // Native DI injects ServiceKey only in a non-null keyed context. + // Unkeyed/null-key parameters use normal services or defaults; named + // overrides above retain priority in both selection and resolution. + if (serviceKey != null && parameter.IsServiceKey) { - if (serviceKey != null && parameter.ParameterType != typeof(object) && parameter.ParameterType != serviceKey.GetType()) + if (parameter.ParameterType != typeof(object) && parameter.ParameterType != serviceKey.GetType()) throw new InvalidOperationException("The ServiceKey parameter type must match the service key type or be object."); return AcceptsValue(parameter.ParameterType, serviceKey); } @@ -191,7 +194,7 @@ bool IsKeyedRegistered(Type type, object? key) return dependency.T == Dependency.DependencyType.KeyedServices ? keyed.GetKeyedService(parameter.ParameterType, dependency.Value) : dependency.Value; - if (parameter.IsServiceKey) return serviceKey; + if (serviceKey != null && parameter.IsServiceKey) return serviceKey; if (!IsRegistered(parameter) && parameter.HasDefaultValue) return parameter.DefaultValue; var key = EffectiveKey(parameter.FromKey, serviceKey);