From e7f4ca65d992e0e80b345900565ecefbcf183fe8 Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Fri, 9 Oct 2026 12:00:11 +0200 Subject: [PATCH 1/5] test: reproduce decorated original graph-planning errors (#121) Co-authored-by: Codex --- ...tensions.Decorators.GraphPlanning.Tests.cs | 201 ++++++++++++++++++ 1 file changed, 201 insertions(+) create mode 100644 src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs new file mode 100644 index 0000000..68a207c --- /dev/null +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs @@ -0,0 +1,201 @@ +using Mammoth.Extensions.DependencyInjection.Configuration; +using Microsoft.Extensions.DependencyInjection; + +namespace Mammoth.Extensions.DependencyInjection.Tests; + +[TestClass] +public class DecoratedGraphPlanningRegressionTests +{ + public static IEnumerable GraphCases() + { + foreach (var type in new[] { typeof(Rejected), typeof(RejectedEnumerable), typeof(Selected), typeof(SelectedEnumerable) }) + foreach (var kind in new[] { "native", "snapshot", "diagnostics" }) + foreach (var lifetime in new[] { ServiceLifetime.Transient, ServiceLifetime.Scoped, ServiceLifetime.Singleton }) + foreach (var keyed in new[] { false, true }) + foreach (var layers in new[] { 1, 2 }) + yield return [type, kind, lifetime, keyed, layers]; + } + + [TestMethod] + [DynamicData(nameof(GraphCases))] + public void OriginalGraphsFailBeforeActivationAndValidGraphsActivateOnlyOnce( + Type type, string kind, ServiceLifetime lifetime, bool keyed, int layers) + { + foreach (var closed in new[] { false, true }) + foreach (var valid in new[] { false, true }) + { + var nativeCounts = new Counts(); + var counts = new Counts(); + var nativeServices = Configure(nativeCounts); + var services = Configure(counts); + using var native = nativeServices.BuildServiceProvider(); + for (var index = 0; index < layers; index++) services.Decorate(); + // Startup validation correctly catches an invalid closed dependency; + // open generic graphs are discovered only at runtime on every provider. + if (closed && !valid && kind == "diagnostics") + { + Assert.ThrowsExactly(() => Build(services, kind)); + Assert.AreEqual(0, counts.Parts + counts.Broken + counts.Roots + counts.Wrappers); + continue; + } + using var provider = Build(services, kind); + // A caller's later mutation must not repair the already-built graph. + services.AddSingleton(); + using var nativeScope = native.CreateScope(); + using var scope = provider.CreateScope(); + for (var repeat = 0; repeat < 2; repeat++) + { + if (!valid) + { + var nativeError = Assert.ThrowsExactly(() => Resolve(nativeScope.ServiceProvider, keyed)); + var error = Assert.ThrowsExactly(() => Resolve(scope.ServiceProvider, keyed)); + StringAssert.Contains(nativeError.Message, nameof(MissingOther)); + StringAssert.Contains(error.Message, nameof(MissingOther)); + Assert.AreEqual(0, nativeCounts.Parts + nativeCounts.Broken + nativeCounts.Roots); + Assert.AreEqual(0, counts.Parts + counts.Broken + counts.Roots + counts.Wrappers, + "Graph errors must be detected before executing any application factories or constructors."); + } + else + { + var expected = Resolve(nativeScope.ServiceProvider, keyed); + var result = Resolve(scope.ServiceProvider, keyed); + Assert.AreEqual(expected.Value, result.Value); + Assert.AreEqual(nativeCounts.Parts, counts.Parts); + Assert.AreEqual(nativeCounts.Broken, counts.Broken); + Assert.AreEqual(nativeCounts.Roots, counts.Roots); + Assert.AreEqual(counts.Roots * layers, counts.Wrappers); + } + } + + IServiceCollection Configure(Counts tracker) + { + var collection = Services(tracker); + if (valid) collection.AddSingleton(); + collection.Add(closed ? ServiceDescriptor.Transient, Broken>() + : ServiceDescriptor.Transient(typeof(IBroken<>), typeof(Broken<>))); + collection.Add(keyed ? ServiceDescriptor.DescribeKeyed(typeof(IWork), "blue", type, lifetime) + : ServiceDescriptor.Describe(typeof(IWork), type, lifetime)); + return collection; + } + } + } + + [TestMethod] + [DataRow("native", false)] + [DataRow("native", true)] + [DataRow("snapshot", false)] + [DataRow("snapshot", true)] + [DataRow("diagnostics", false)] + [DataRow("diagnostics", true)] + public void NonemptyDependsOnFactoriesKeepTheirSeparateRejectedCandidatePolicy(string kind, bool keyed) + { + var counts = new Counts(); + var services = Services(counts); + services.AddTransient(typeof(IBroken<>), typeof(Broken<>)); + Dependency[] map = [Dependency.OnValue("unused", 1)]; + if (keyed) services.AddKeyedTransient("blue", map); + else services.AddTransient(map); + services.Decorate(); + using var provider = Build(services, kind); + using var scope = provider.CreateScope(); + Assert.AreEqual("valid", Resolve(scope.ServiceProvider, keyed).Value); + Assert.AreEqual(1, counts.Parts); + Assert.AreEqual(0, counts.Broken); + } + + [TestMethod] + [DataRow("native", false)] + [DataRow("native", true)] + [DataRow("snapshot", false)] + [DataRow("snapshot", true)] + [DataRow("diagnostics", false)] + [DataRow("diagnostics", true)] + public void OpaqueDependencyFactoriesStillRunOnlyAtRealResolutionAndKeepTheirException(string kind, bool keyed) + { + var nativeCounts = new Counts(); + var counts = new Counts(); + var failure = new InvalidOperationException("Application dependency failure"); + using var native = Configure(nativeCounts).BuildServiceProvider(); + var services = Configure(counts); + services.Decorate(); + using var provider = Build(services, kind); + using var nativeScope = native.CreateScope(); + using var scope = provider.CreateScope(); + Assert.AreSame(failure, Assert.ThrowsExactly(() => Resolve(nativeScope.ServiceProvider, keyed))); + Assert.AreSame(failure, Assert.ThrowsExactly(() => Resolve(scope.ServiceProvider, keyed))); + Assert.AreEqual(1, counts.Parts); + Assert.AreEqual(nativeCounts.Parts, counts.Parts); + Assert.AreEqual(1, counts.Broken); + Assert.AreEqual(nativeCounts.Broken, counts.Broken); + Assert.AreEqual(0, counts.Roots + counts.Wrappers); + + IServiceCollection Configure(Counts tracker) + { + var collection = Services(tracker); + collection.AddTransient>(_ => { tracker.Broken++; throw failure; }); + if (keyed) collection.AddKeyedTransient("blue"); + else collection.AddTransient(); + return collection; + } + } + + private static IServiceCollection Services(Counts counts) + { + IServiceCollection services = new ServiceCollection(); + services.AddSingleton(counts); + services.AddTransient(_ => { counts.Parts++; return new Part(); }); + services.AddSingleton(); + return services; + } + private static IWork Resolve(IServiceProvider provider, bool keyed) => keyed + ? provider.GetRequiredKeyedService("blue") : 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 Counts { public int Parts; public int Broken; public int Roots; public int Wrappers; } + public sealed class Part; + public sealed class Other; + public sealed class Missing; + public sealed class MissingOther; + public interface IBroken; + public sealed class Broken : IBroken + { + public Broken(MissingOther missing, Counts counts) => counts.Broken++; + } + public interface IWork { string Value { get; } } + public abstract class Work : IWork + { + public string Value { get; } + protected Work(string value, Counts counts) { Value = value; counts.Roots++; } + } + public sealed class Rejected : Work + { + public Rejected(Part part, Other other, Counts counts) : base("valid", counts) { } + public Rejected(IBroken broken, Missing missing, Counts counts) : base("invalid", counts) { } + } + public sealed class RejectedEnumerable : Work + { + public RejectedEnumerable(Part part, Other other, Counts counts) : base("valid", counts) { } + public RejectedEnumerable(IEnumerable> broken, Missing missing, Counts counts) : base("invalid", counts) { } + } + public sealed class Selected(Part part, IBroken broken, Counts counts) : Work("selected", counts) + { + public Part Part { get; } = part; + public IBroken Broken { get; } = broken; + } + public sealed class SelectedEnumerable(Part part, IEnumerable> broken, Counts counts) : Work("selected", counts) + { + public Part Part { get; } = part; + public IEnumerable> Broken { get; } = broken; + } + public sealed class Forwarder : IWork + { + public IWork Inner { get; } + public string Value => Inner.Value; + public Forwarder(IWork inner, Counts counts) { Inner = inner; counts.Wrappers++; } + } +} From 1b504e3f4263f26372fce3cdce9b5c6fd30a5444 Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Fri, 9 Oct 2026 12:06:55 +0200 Subject: [PATCH 2/5] test: guard enumerable members and graph validation provider isolation Co-authored-by: Codex --- ...tensions.Decorators.GraphPlanning.Tests.cs | 70 +++++++++++++++++++ 1 file changed, 70 insertions(+) diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs index 68a207c..ff2d286 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs @@ -139,6 +139,76 @@ IServiceCollection Configure(Counts tracker) } } + [TestMethod] + [DataRow("native", false)] + [DataRow("native", true)] + [DataRow("snapshot", false)] + [DataRow("snapshot", true)] + [DataRow("diagnostics", false)] + [DataRow("diagnostics", true)] + public void EarlierInvalidEnumerableRegistrationIsNotHiddenByLastValidBinding(string kind, bool keyed) + { + var nativeCounts = new Counts(); + var counts = new Counts(); + using var native = Configure(nativeCounts).BuildServiceProvider(); + var services = Configure(counts); + services.Decorate(); + services.Decorate(); + using var provider = Build(services, kind); + using var scope = provider.CreateScope(); + StringAssert.Contains(Assert.ThrowsExactly(() => Resolve(native, keyed)).Message, nameof(MissingOther)); + StringAssert.Contains(Assert.ThrowsExactly(() => Resolve(scope.ServiceProvider, keyed)).Message, nameof(MissingOther)); + Assert.AreEqual(0, nativeCounts.Parts + nativeCounts.Broken + nativeCounts.Roots); + Assert.AreEqual(0, counts.Parts + counts.Broken + counts.Roots + counts.Wrappers); + + IServiceCollection Configure(Counts tracker) + { + var collection = Services(tracker); + collection.AddTransient(typeof(IBroken<>), typeof(Broken<>)); + collection.AddTransient(typeof(IBroken<>), typeof(Healthy<>)); + if (keyed) collection.AddKeyedTransient("blue"); + else collection.AddTransient(); + return collection; + } + } + + [TestMethod] + [DataRow("native", false)] + [DataRow("native", true)] + [DataRow("snapshot", false)] + [DataRow("snapshot", true)] + [DataRow("diagnostics", false)] + [DataRow("diagnostics", true)] + public void SuccessfulGraphCheckDoesNotLeakAcrossProvidersSharingTheOriginalDescriptor(string kind, bool keyed) + { + var original = keyed ? ServiceDescriptor.KeyedTransient("blue") + : ServiceDescriptor.Transient(); + var validCounts = new Counts(); + var invalidCounts = new Counts(); + using var valid = Build(Configure(validCounts, complete: true), kind); + using var invalid = Build(Configure(invalidCounts, complete: false), kind); + using var validScope = valid.CreateScope(); + using var invalidScope = invalid.CreateScope(); + Assert.AreEqual("selected", Resolve(validScope.ServiceProvider, keyed).Value); + StringAssert.Contains(Assert.ThrowsExactly(() => Resolve(invalidScope.ServiceProvider, keyed)).Message, nameof(MissingOther)); + Assert.AreEqual(0, invalidCounts.Parts + invalidCounts.Broken + invalidCounts.Roots + invalidCounts.Wrappers); + + IServiceCollection Configure(Counts tracker, bool complete) + { + var collection = Services(tracker); + if (complete) collection.AddSingleton(); + collection.AddTransient(typeof(IBroken<>), typeof(Broken<>)); + collection.Add(original); + collection.Decorate(); + return collection; + } + } + + public sealed class Healthy : IBroken + { + public Healthy(Counts counts) => counts.Broken++; + } + private static IServiceCollection Services(Counts counts) { IServiceCollection services = new ServiceCollection(); From c4108b89c93cee2f3248a087ab3a8ba8fc80af47 Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Fri, 9 Oct 2026 17:02:25 +0200 Subject: [PATCH 3/5] Restore native constructor graphs for decorated originals Preserve keys and native ownership, with a runtime scoped holder for compiled caches. Update differential coverage and consumer guidance. Co-authored-by: Codex --- .agents/skills/mammoth-di/SKILL.md | 6 +- .agents/skills/mammoth-di/references/usage.md | 2 +- Changelog.md | 2 + README.md | 4 +- docs/keyed-built-in-registration-probing.md | 14 +- docs/original-decoration-graph-planning.md | 27 +++ ...ntDisposables.DecoratorExclusions.Tests.cs | 3 +- ...tensions.Decorators.GraphPlanning.Tests.cs | 28 ++- ...ons.Decorators.NativeConstructors.Tests.cs | 23 +++ ...ensions.Decorators.NativeIdentity.Tests.cs | 164 ++++++++++++++++++ ...tensions.Decorators.OrdinaryProbe.Tests.cs | 25 +-- ...lectionExtensions.KeyedActivation.Tests.cs | 6 +- ...ProviderFactory.DecoratorMetadata.Tests.cs | 18 +- ...erFactory.GenericLifetimeSnapshot.Tests.cs | 6 +- ...oviderFactory.OpenGenericSnapshot.Tests.cs | 7 +- .../DecorationServiceType.cs | 15 ++ ...ectIncorrectUsageOfTransientDisposables.cs | 30 ++++ .../ServiceCollectionExtensions.Decorators.cs | 42 ++++- ...erviceCollectionExtensions.Registration.cs | 7 +- .../ServiceProviderFactory.cs | 3 + test-support/NativeResolverCompilation.cs | 3 + 21 files changed, 385 insertions(+), 50 deletions(-) create mode 100644 docs/original-decoration-graph-planning.md create mode 100644 src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeIdentity.Tests.cs create mode 100644 src/Mammoth.Extensions.DependencyInjection/DecorationServiceType.cs diff --git a/.agents/skills/mammoth-di/SKILL.md b/.agents/skills/mammoth-di/SKILL.md index 2ac6325..1e6f081 100644 --- a/.agents/skills/mammoth-di/SKILL.md +++ b/.agents/skills/mammoth-di/SKILL.md @@ -9,7 +9,7 @@ Use the application's existing service collection/host and requested lifetimes. ## Establish the applicable contract -Read the consumer's PackageReference/central package versions and resolved assets before assuming these guarantees. This skill describes current develop after the disposal, constructor, snapshot, keyed-generic and DI-cache fixes. They are **unreleased** as of October 1, 2026; latest listed stable release is 0.7.1. Don't assume an unversioned package install includes them. When the app is pinned to older behavior, explain the needed version/fix and respect its upgrade constraints rather than silently changing a major dependency. +Read the consumer's PackageReference/central package versions and resolved assets before assuming these guarantees. This skill describes current develop targeting the next release after published 0.8.0. Constructor-graph restoration (#121), attribute-order fixes (#122) and the vNext changes are **unreleased**; do not assume 0.8.0 includes them. Don't assume an unversioned package install includes them. When the app is pinned to older behavior, explain the needed version/fix and respect its upgrade constraints rather than silently changing a major dependency. The fixed baseline requires Microsoft DI >=10.0.0 and Bcl.AsyncInterfaces >=10.0.0. DI 9.0.0/9.0.20 can corrupt reused keyed/unkeyed enumeration after native compilation. Use a release containing the upstream fix; fresh providers are not a workaround. Targets remain netstandard2.0/net8/net9/net10, verified on net472 Windows and .NET 8/9/10. netstandard is a compile contract, not a runtime. Adapt example C# syntax to the application's SDK. @@ -17,9 +17,9 @@ Imports: `Microsoft.Extensions.DependencyInjection`, `Mammoth.Extensions.Depende ## Choose registration and resolution deliberately -- Decorate with `Decorate()` after the intended registration. It wraps only the last exact service registration, preserving its key/lifetime/order; repeat calls nest outer layers. Container-created inner services and decorators are disposed independently. Never forward decorator disposal to the injected inner; caller-supplied instances stay caller-owned. This closed generic API does not register open-generic decorators. +- Decorate with `Decorate()` after the intended registration. It wraps only the last exact service registration, preserving its key/lifetime/order; repeat calls nest outer layers. Container-created inner services and decorators are disposed independently. Never forward decorator disposal to the injected inner; caller-supplied instances stay caller-owned. This closed generic API does not register open-generic decorators. Original type registrations retain native graph validation before activation, including rejected constructor candidates and enumerable members; `ValidateOnBuild` may reject them at startup. Factory originals and non-empty maps remain opaque to that inspection. - Map **parameter names** using `Parameter.ForKey("store").Eq("primary")` or `Dependency.OnValue("label", "batch")`. `Eq` accepts a string key; other native keys can use a suitable attribute constant or an explicit factory. Values must be assignable; maps are not global registrations. Unused names are ignored, so verify names against the constructor. -- Non-empty DependsOn maps select at resolution: one preferred `[ActivatorUtilitiesConstructor]` 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 services; optional defaults apply only to unregistered dependencies. Use explicit attribute keys on this mapped path; do not assume newer native attribute lookup modes are supported just because DI 10 is referenced. Rejected constructors' dependencies are not created. Empty maps delegate to native DI. Custom providers need keyed resolution plus ordinary/keyed service probes. +- Non-empty DependsOn maps select at resolution: one preferred `[ActivatorUtilitiesConstructor]` 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 services; optional defaults apply only to unregistered dependencies. `[FromKeyedServices]` inherits the current key; `[FromKeyedServices(null)]` performs unkeyed lookup. `[ServiceKey]` only injects a non-null current key. For both attributes on one untouched parameter, metadata order selects the first effective non-null binding; named maps override both. Rejected constructors' dependencies are not created. Empty maps delegate to native DI. Mapped factories need keyed resolution plus ordinary/keyed service probes. Native original type registrations behind decorators need no separate keyed availability probe; contextual decorator constructors retain their probes. - Build with `ServiceProviderFactory.CreateServiceProvider(services, options)` when using Mammoth provider queries/discovery. For a host, use its `UseServiceProviderFactory` hook rather than constructing a second provider. Native BuildServiceProvider alone does not create Mammoth's authoritative metadata. For assembly scanning, constrain `AssemblyInspector` filters and assign maps through `Configure((registration, type) => registration.DependsOn = ...)` before the parameterless `LifestyleSingleton`/Scoped/Transient method. Do not invent a lifestyle overload taking a map. diff --git a/.agents/skills/mammoth-di/references/usage.md b/.agents/skills/mammoth-di/references/usage.md index 5279ae0..83b4318 100644 --- a/.agents/skills/mammoth-di/references/usage.md +++ b/.agents/skills/mammoth-di/references/usage.md @@ -151,7 +151,7 @@ public sealed class Scanned(string label) { public string Label { get; } = label ## Keyed decorators and caller-owned instances -Decorate the intended keyed registration immediately, before adding another registration of that service type. Each wrapper releases only its own resources. A caller-owned singleton is explicitly disposed by the caller after the provider releases its wrapper. +Decorate the intended keyed registration immediately, before adding another registration of that service type. Native original type graphs are validated before activation; enabling `ValidateOnBuild` can move graph errors to startup. Factory originals and non-empty maps retain their own activation policies. Each wrapper releases only its own resources. A caller-owned singleton is explicitly disposed by the caller after the provider releases its wrapper. ```csharp using Microsoft.Extensions.DependencyInjection; diff --git a/Changelog.md b/Changelog.md index 569d38c..879486a 100644 --- a/Changelog.md +++ b/Changelog.md @@ -10,6 +10,8 @@ ### Bug Fixes +- Restore native constructor graph validation for decorated implementation-type originals, rejecting invalid nested/generic/enumerable/cyclic graphs before dependency factories execute. Preserve original keys, independent registration caches and exactly-once disposal; keep scoped caching correct after native compilation [#121](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/121). + - Preserve native metadata order when FromKeyedServices and ServiceKey share a constructor parameter. Keep named DependsOn overrides first, respect effective null/inherited keys, and apply the same binding in selection and resolution without additional service lookups [#122](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/122). - 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). diff --git a/README.md b/README.md index 01a81a0..11d250a 100644 --- a/README.md +++ b/README.md @@ -99,6 +99,8 @@ services.Decorate(); Decorators preserve Singleton, Scoped and Transient lifetimes and native keys for type, instance and factory registrations. `Decorate()` wraps only the **last exact service-type registration**, retaining its position. Repeated calls add outer layers; this API does not register open-generic decorators. +Original implementation-type registrations retain native constructor graph planning: invalid nested, generic, enumerable or cyclic graphs fail before dependency factories run. `ValidateOnBuild` can reject them at startup, including inherited-key dependencies validated under `AnyKey`. Factory registrations and non-empty dependency maps remain opaque to native graph inspection. See [the registration design](docs/original-decoration-graph-planning.md). + Each occurrence in the collection is a separate registration, even when the same `ServiceDescriptor` instance is added more than once. Only the last occurrence is decorated: ```csharp @@ -187,7 +189,7 @@ The current design is limited to some common use case and it's very similar to t This extension supports Singleton, Scoped, Transient and Keyed registrations. Maps match constructor **parameter names**, not service types; `Eq` accepts string keys and mapped values must be assignable. Unused map names are ignored, so check names carefully. -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. +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. Mapped activation needs ordinary/keyed service probes and keyed resolution. `[FromKeyedServices]` inherits the current key; `[FromKeyedServices(null)]` uses unkeyed lookup. With `AnyKey` registrations, resolve a concrete key when inheriting dependencies. Ordinary decoration needs no keyed availability probe for native original type activation; contextual decorator constructors and mapped factories keep their probe requirements. `[ServiceKey]` injects only a non-null service key. In unkeyed or null-key registrations it is ignored, allowing another effective keyed attribute, an ordinary service or an optional default to bind the parameter. When both `[FromKeyedServices]` and `[ServiceKey]` appear on an untouched parameter, metadata order determines the first effective non-null key binding; a null-key lookup does not suppress a later `[ServiceKey]`. Named overrides still take precedence. diff --git a/docs/keyed-built-in-registration-probing.md b/docs/keyed-built-in-registration-probing.md index 1e11ff2..b37efa1 100644 --- a/docs/keyed-built-in-registration-probing.md +++ b/docs/keyed-built-in-registration-probing.md @@ -63,19 +63,13 @@ Before reading the descriptor array, the code verifies that the field exists and This explanation concerns the new private-DI registration fallback. Constructor/parameter reflection used by the existing activator is a separate mechanism. -## Original type activation in decorators +## Native original types and diagnostic activation -[Issue #111](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/111) reuses the native-compatible activator for original implementation-type descriptors hidden by decoration. It ignores `ActivatorUtilitiesConstructorAttribute`, accepts satisfiable constructor parameter permutations/subsets, and rejects unrelated parameter-type sets before activating dependencies. New decorators and nonempty DependsOn factories keep their existing construction policies. +[Issue #121](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/121) keeps decorated original implementation types in native DI registrations. Native DI now handles their constructor selection, generic constraints and complete inspectable dependency graphs; this path no longer calls Mammoth's original-type activator or its copied-descriptor fallback. Private identities retain the original public key and isolate each registration. See [the registration design](original-decoration-graph-planning.md), including the scoped compiled-cache safeguard. -This activator uses the same keyed built-in availability helper described above. It also validates open-generic constraints for every examined constructor candidate, including candidates that would not ultimately be selected. Native DI performs that validation while constructing argument call sites; its public availability probes report a registration without testing the implementation's generic constraints. Resolving the dependency to check constraints would activate factories too early. +Diagnostic instrumentation still uses `NativeConstructorActivator` for ordinary implementation types after native startup validation. It ignores preferred-constructor attributes and checks parameter-type ambiguity and generic constraints without activating rejected dependencies. Mammoth snapshots are preferred; recognized native ordinary probes reuse the guarded descriptor fallback and provider-specific weak cache. It requests a keyed availability probe only for non-null dependency lookup keys (#120). This fallback adds no private member lookup beyond the descriptor field described above. -Mammoth providers supply their existing registration snapshot. For a recognized native probe, the guarded `ReadNativeDescriptors` helper reads the same copied descriptor array through the existing cached field lookup, then builds an internal snapshot cached separately in a `ConditionalWeakTable`. This additional snapshot retains descriptor identities, lifetimes and implementation metadata needed for constraint validation. The keyed-built-in identity cache still stores only the four built-in types. Both caches are provider-specific and weakly keyed; neither reads the subsequently mutable caller collection or activates a service. Exact closed registrations, including AnyKey, take precedence over open-generic bindings. - -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. +New decorators and non-empty DependsOn maps retain their separate construction and probe policies. Contextual decorator constructors and mapped activation need the keyed availability probe; native original types behind decoration use the provider's native call sites directly. ## Verification of the compatibility choice diff --git a/docs/original-decoration-graph-planning.md b/docs/original-decoration-graph-planning.md new file mode 100644 index 0000000..c36eab0 --- /dev/null +++ b/docs/original-decoration-graph-planning.md @@ -0,0 +1,27 @@ +# Native graph planning for decorated originals + +Issue #121 regressed from 0.7.1 when #37 moved original services behind private factories to fix disposal ownership. Factories hid constructor graphs: invalid rejected candidates could be skipped, and selected graphs could activate an earlier dependency factory before failing. + +## Registration design + +`RegisterInnerLayer` keeps original implementation types as native DI type registrations. Each receives a private `DecorationServiceType`, a public `TypeDelegator` over `object` with reference identity and explicit object assignability. `UnderlyingSystemType` retains that private identity, so normal object registrations compare unequal. The original public service key is preserved, including unkeyed/null-key context, rather than replacing it with a private key. + +Native DI plans constructors and dependency call sites before constructing the original. It owns that original and the public decorator independently. Caller-supplied instances remain caller-owned. Factory/map originals and preceding decorators retain the existing private factory slots and activation policy. No extra provider is built for runtime graph checking, and no application factory is used as a validation probe. + +## Scoped compiled caching + +The [native DI 10 IL emitter](https://github.com/dotnet/runtime/blob/v10.0.0/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ILEmit/ILEmitResolverBuilder.cs#L248-L256) reconstructs scoped cache keys using a runtime type token. A delegated identity cannot survive that operation. A scoped original therefore uses a transient native type registration plus a scoped, non-disposable holder under a normal private runtime marker type. Native DI creates/captures the inner in that scope; the holder caches its reference without capturing disposal a second time. Singleton and transient originals keep their native lifetimes. Public service and decorator lifetimes are unchanged. + +The native original and scoped holder are both hidden from public Mammoth discovery. Keys remain public keys, so a key-only query can still report the original registration's key. Private descriptor lifetimes are implementation details; the public service lifetime stays authoritative. + +## Diagnostics and validation timing + +Diagnostics preserve the native type descriptor and track resolution through the public decorator factory. A small per-provider options service lets that factory check transient disposable originals before creation, with the existing exclusion and singleton-ancestor rules. The shared caller collection is never reconfigured per provider. + +`ValidateOnBuild` can now reject a decorated original at startup, just as it rejects the corresponding undecorated native type. This includes ambiguity, invalid generic constraints and `AnyKey` inherited dependencies that have no wildcard binding at validation time. Use a valid wildcard dependency or concrete registrations when startup validation needs one. Disabling validation for diagnostics remains unsupported. + +Factories, non-empty dependency maps and decorator constructors are opaque to native graph planning. This restores native original-type behavior; it does not promise inspection through arbitrary application factories or change DependsOn's separate selection policy. + +## Regression evidence + +Differential tests compare native and decorated registrations under the same validation settings, require zero factory/constructor calls on graph failures, and cover rejected/selected constructors, open/closed generics, enumerable ordering, cycles, keys, lifetimes, repeated layers and provider isolation. Native-only test instrumentation observes compiled accessor replacement for originals and scoped holders, then checks 100 repeated resolutions, independent caches, ordinary object/implementation registrations and exactly-once disposal across all supported test targets. Instrumentation is never included in the production library. diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposables.DecoratorExclusions.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposables.DecoratorExclusions.Tests.cs index 3151abe..a37fcb9 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposables.DecoratorExclusions.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/DetectIncorrectUsageOfTransientDisposables.DecoratorExclusions.Tests.cs @@ -40,7 +40,8 @@ public void ExactPublicExclusionSurvivesEveryLayerAndPreservesOwnership(bool key Assert.IsFalse(provider.IsServiceRegistered(layer.ServiceType)); Assert.HasCount(0, provider.GetAllServices(layer.ServiceType).ToArray()); Assert.IsFalse(provider.GetRequiredService().Contains(layer.ServiceType)); - Assert.IsFalse(provider.GetRequiredService().Contains(layer.ServiceKey!)); + Assert.AreEqual(layer.ServiceKey != null && Equals(layer.ServiceKey, key), + provider.GetRequiredService().Contains(layer.ServiceKey!)); } var discovered = provider.GetAllServices(serviceType).ToArray(); Assert.HasCount(1, discovered); diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs index ff2d286..b6096f7 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs @@ -30,11 +30,15 @@ public void OriginalGraphsFailBeforeActivationAndValidGraphsActivateOnlyOnce( var services = Configure(counts); using var native = nativeServices.BuildServiceProvider(); for (var index = 0; index < layers; index++) services.Decorate(); - // Startup validation correctly catches an invalid closed dependency; - // open generic graphs are discovered only at runtime on every provider. - if (closed && !valid && kind == "diagnostics") + // Native type originals remain visible to ValidateOnBuild, which + // also closes their open-generic dependencies while planning the root. + if (!valid && kind == "diagnostics") { - Assert.ThrowsExactly(() => Build(services, kind)); + var nativeError = Assert.ThrowsExactly(() => nativeServices.BuildServiceProvider( + new ServiceProviderOptions { ValidateOnBuild = true })); + var error = Assert.ThrowsExactly(() => Build(services, kind)); + StringAssert.Contains(nativeError.ToString(), nameof(MissingOther)); + StringAssert.Contains(error.ToString(), nameof(MissingOther)); Assert.AreEqual(0, counts.Parts + counts.Broken + counts.Roots + counts.Wrappers); continue; } @@ -154,6 +158,14 @@ public void EarlierInvalidEnumerableRegistrationIsNotHiddenByLastValidBinding(st var services = Configure(counts); services.Decorate(); services.Decorate(); + if (kind == "diagnostics") + { + Assert.ThrowsExactly(() => Configure(nativeCounts).BuildServiceProvider( + new ServiceProviderOptions { ValidateOnBuild = true })); + StringAssert.Contains(Assert.ThrowsExactly(() => Build(services, kind)).ToString(), nameof(MissingOther)); + Assert.AreEqual(0, counts.Parts + counts.Broken + counts.Roots + counts.Wrappers); + return; + } using var provider = Build(services, kind); using var scope = provider.CreateScope(); StringAssert.Contains(Assert.ThrowsExactly(() => Resolve(native, keyed)).Message, nameof(MissingOther)); @@ -186,6 +198,14 @@ public void SuccessfulGraphCheckDoesNotLeakAcrossProvidersSharingTheOriginalDesc var validCounts = new Counts(); var invalidCounts = new Counts(); using var valid = Build(Configure(validCounts, complete: true), kind); + if (kind == "diagnostics") + { + StringAssert.Contains(Assert.ThrowsExactly(() => Build(Configure(invalidCounts, complete: false), kind)).ToString(), nameof(MissingOther)); + Assert.AreEqual(0, invalidCounts.Parts + invalidCounts.Broken + invalidCounts.Roots + invalidCounts.Wrappers); + using var validOnlyScope = valid.CreateScope(); + Assert.AreEqual("selected", Resolve(validOnlyScope.ServiceProvider, keyed).Value); + return; + } using var invalid = Build(Configure(invalidCounts, complete: false), kind); using var validScope = valid.CreateScope(); using var invalidScope = invalid.CreateScope(); diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeConstructors.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeConstructors.Tests.cs index 9c7b127..1a78758 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeConstructors.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeConstructors.Tests.cs @@ -31,6 +31,18 @@ public void OriginalTypeKeepsNativeConstructorRulesAcrossLayersAndLifetimes( Add(services, type, lifetime, keyed); services.Decorate(); services.Decorate(); + if (kind == "diagnostics" && (type == typeof(DifferentParameters) || type == typeof(EqualLengthDifferentParameters))) + { + var nativeError = Assert.ThrowsExactly(() => nativeServices.BuildServiceProvider( + new ServiceProviderOptions { ValidateOnBuild = true })); + var error = Assert.ThrowsExactly(() => Build(services, kind)); + StringAssert.Contains(nativeError.ToString(), "ambiguous"); + StringAssert.Contains(error.ToString(), "ambiguous"); + Assert.AreEqual(0, nativeCounts.Dependencies); + Assert.AreEqual(0, counts.Dependencies); + Assert.AreEqual(0, counts.Constructors); + return; + } using var native = nativeServices.BuildServiceProvider(); using var provider = Build(services, kind); Assert.AreEqual(0, counts.Dependencies); @@ -125,6 +137,17 @@ public void NativeProvidersPreserveBuiltInAvailabilityAndGenericConstraintChecks var services = Configure(counts); using var native = nativeServices.BuildServiceProvider(); services.Decorate(); + if (kind == "diagnostics" && scenario == "generic-invalid") + { + Assert.ThrowsExactly(() => nativeServices.BuildServiceProvider( + new ServiceProviderOptions { ValidateOnBuild = true })); + var error = Assert.ThrowsExactly(() => Build(services, kind)); + Assert.IsInstanceOfType(error.InnerExceptions[0].InnerException); + Assert.AreEqual(0, nativeCounts.Dependencies); + Assert.AreEqual(0, counts.Dependencies); + Assert.AreEqual(0, counts.Constructors); + return; + } using var provider = Build(services, kind); // Mutating the caller collection must not replace a built provider's metadata. services.AddKeyedTransient, Open>("bad"); diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeIdentity.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeIdentity.Tests.cs new file mode 100644 index 0000000..c3ddea9 --- /dev/null +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeIdentity.Tests.cs @@ -0,0 +1,164 @@ +using Mammoth.DependencyInjection.Regression; +using Microsoft.Extensions.DependencyInjection; + +namespace Mammoth.Extensions.DependencyInjection.Tests; + +[TestClass] +public class DecoratedNativeIdentityTests +{ + [TestMethod] + [DataRow(ServiceLifetime.Transient, false)] + [DataRow(ServiceLifetime.Transient, true)] + [DataRow(ServiceLifetime.Scoped, false)] + [DataRow(ServiceLifetime.Scoped, true)] + public void NativeOriginalsCompileWithoutCollidingWithOtherRegistrations(ServiceLifetime lifetime, bool keyed) + { + IServiceCollection services = new ServiceCollection(); + var counts = new Counts(); + var ordinaryObject = new object(); + services.AddSingleton(counts); + services.AddSingleton(ordinaryObject); + services.AddTransient(); + object? key = keyed ? "blue" : null; + for (var i = 0; i < 2; i++) + { + services.Add(keyed ? ServiceDescriptor.DescribeKeyed(typeof(IWork), key, typeof(Original), lifetime) + : ServiceDescriptor.Describe(typeof(IWork), typeof(Original), lifetime)); + services.Decorate(); + } + var aliases = services.Where(d => (d.IsKeyedService ? d.KeyedImplementationType : d.ImplementationType) == typeof(Original) + && d.ServiceType != typeof(Original)).ToArray(); + Assert.HasCount(2, aliases); + Assert.IsFalse(aliases[0].ServiceType.Equals(aliases[1].ServiceType)); + Assert.IsFalse(typeof(object).Equals(aliases[0].ServiceType)); + Assert.IsFalse(aliases[0].ServiceType.Equals(typeof(object))); + Assert.IsTrue(aliases[0].ServiceType.IsAssignableFrom(aliases[0].ServiceType)); + using var provider = ServiceProviderFactory.CreateServiceProvider(services); + using var scope = provider.CreateScope(); + var first = Resolve(scope.ServiceProvider); + var compiledDescriptors = aliases.Concat(services.Where(d => d.Lifetime == ServiceLifetime.Scoped && d.ServiceType != typeof(IWork))).ToArray(); + var observers = compiledDescriptors.Select(d => NativeResolverCompilation.ObserveService(provider, d.ServiceType, key)).ToArray(); + // Cached scoped public decorators do not call their originals again. + // Resolve the private descriptors directly to trigger both native compilers. + foreach (var descriptor in compiledDescriptors) + scope.ServiceProvider.GetRequiredKeyedService(descriptor.ServiceType, key); + foreach (var observer in observers) observer.WaitForReplacement(); + for (var repeat = 0; repeat < 100; repeat++) + { + var result = Resolve(scope.ServiceProvider); + Assert.HasCount(2, result); + Assert.AreNotSame(result[0].Inner, result[1].Inner); + for (var index = 0; index < 2; index++) + { + Assert.AreEqual(keyed ? key : ordinaryObject, result[index].Inner.Key); + if (lifetime == ServiceLifetime.Scoped) Assert.AreSame(first[index].Inner, result[index].Inner); + else Assert.AreNotSame(first[index].Inner, result[index].Inner); + } + Assert.AreSame(ordinaryObject, scope.ServiceProvider.GetRequiredService()); + Assert.AreNotSame(result[0].Inner, scope.ServiceProvider.GetRequiredService()); + } + using (var other = provider.CreateScope()) + { + var result = Resolve(other.ServiceProvider); + Assert.AreNotSame(first[0].Inner, result[0].Inner); + Assert.AreNotSame(first[1].Inner, result[1].Inner); + } + scope.Dispose(); + Assert.IsNotEmpty(counts.Created); + Assert.IsTrue(counts.Created.All(original => original.Disposals == 1)); + foreach (var alias in aliases) + { + Assert.IsFalse(provider.IsServiceRegistered(alias.ServiceType)); + Assert.HasCount(0, services.GetServiceDescriptors(alias.ServiceType).ToArray()); + Assert.IsFalse(services.IsTransientServiceRegistered(alias.ServiceType)); + Assert.IsFalse(services.IsScopedServiceRegistered(alias.ServiceType)); + Assert.IsFalse(services.IsSingletonServiceRegistered(alias.ServiceType)); + } + + Wrapper[] Resolve(IServiceProvider sp) => (keyed ? sp.GetKeyedServices(key) : sp.GetServices()).Cast().ToArray(); + } + + [TestMethod] + [DataRow(false, false)] + [DataRow(false, true)] + [DataRow(true, false)] + [DataRow(true, true)] + public void CyclicOriginalGraphsFailBeforeDependencyFactories(bool keyed, bool validateOnBuild) + { + var nativeCounts = new Counts(); + var counts = new Counts(); + var native = Configure(nativeCounts); + var decorated = Configure(counts); + decorated.Decorate(); + var options = new ServiceProviderOptions { ValidateOnBuild = validateOnBuild }; + Exception nativeError, error; + if (validateOnBuild) + { + nativeError = Assert.ThrowsExactly(() => native.BuildServiceProvider(options)); + error = Assert.ThrowsExactly(() => decorated.BuildServiceProvider(options)); + } + else + { + using var nativeProvider = native.BuildServiceProvider(options); + using var provider = decorated.BuildServiceProvider(options); + nativeError = Assert.ThrowsExactly(() => Resolve(nativeProvider)); + error = Assert.ThrowsExactly(() => Resolve(provider)); + } + StringAssert.Contains(nativeError.ToString(), "circular dependency"); + StringAssert.Contains(error.ToString(), "circular dependency"); + Assert.AreEqual(0, nativeCounts.Parts); + Assert.AreEqual(0, counts.Parts); + Assert.HasCount(0, counts.Created); + + IServiceCollection Configure(Counts tracker) + { + IServiceCollection sc = new ServiceCollection(); + sc.AddTransient(_ => { tracker.Parts++; return new Part(); }); + sc.AddTransient(typeof(ICycle<>), typeof(Cycle<>)); + sc.Add(keyed ? ServiceDescriptor.KeyedTransient("blue") + : ServiceDescriptor.Transient()); + return sc; + } + IWork Resolve(IServiceProvider sp) => keyed ? sp.GetRequiredKeyedService("blue") : sp.GetRequiredService(); + } + + [TestMethod] + [DataRow(false)] + [DataRow(true)] + public void DecoratorKeyedDependenciesStillRequireTheirAvailabilityProbe(bool exposeKeyedProbe) + { + var counts = new OrdinaryDecorationProbeRegressionTests.Counts(); + IServiceCollection services = new ServiceCollection(); + services.AddKeyedTransient("blue"); + services.AddKeyedSingleton("blue", new Part()); + services.Decorate(); + var outer = services.Single(d => d.ServiceType == typeof(IWork)); + services[services.IndexOf(outer)] = ServiceDescriptor.DescribeKeyed(typeof(IWork), "blue", + (sp, key) => outer.KeyedImplementationFactory!(new OrdinaryDecorationProbeRegressionTests.ProbeProvider(sp, counts, exposeKeyedProbe, true), key), outer.Lifetime); + using var provider = services.BuildServiceProvider(); + if (exposeKeyedProbe) Assert.IsInstanceOfType(provider.GetRequiredKeyedService("blue")); + else + { + var error = Assert.ThrowsExactly(() => provider.GetRequiredKeyedService("blue")); + StringAssert.Contains(error.Message, nameof(IServiceProviderIsKeyedService)); + } + Assert.IsGreaterThan(0, counts.KeyProbeRequests); + } + + public interface IWork { object? Key { get; } } + public sealed class Counts { public int Parts; public List Created { get; } = []; } + public sealed class Original : IWork, IDisposable + { + public Original(Counts counts, [ServiceKey] object? key = null) { Key = key; counts.Created.Add(this); } + public object? Key { get; } + public int Disposals { get; private set; } + public void Dispose() => Disposals++; + } + public sealed class Wrapper(IWork inner) : IWork { public IWork Inner { get; } = inner; public object? Key => Inner.Key; } + public sealed class Part; + public interface ICycle; + public sealed class Cycle(ICycle self) : ICycle { public ICycle Self { get; } = self; } + public sealed class CyclicOriginal(Part part, ICycle cycle) : IWork { public object? Key => null; public Part Part { get; } = part; public ICycle Cycle { get; } = cycle; } + public sealed class PlainOriginal : IWork { public object? Key => null; } + public sealed class KeyedWrapper(IWork inner, [FromKeyedServices] Part part) : IWork { public object? Key => inner.Key; public Part Part { get; } = part; } +} diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.OrdinaryProbe.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.OrdinaryProbe.Tests.cs index a74562c..22804a3 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.OrdinaryProbe.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.OrdinaryProbe.Tests.cs @@ -86,6 +86,18 @@ public void HiddenKeyedProbeKeepsNativeConstructorPoliciesAndGenericConstraintPl using var native = nativeServices.BuildServiceProvider(); services.Decorate(); WrapFactories(services, counts, exposeKeyedProbe: false, ordinaryOnlyProbe: false); + if (kind == "diagnostics" && (type == typeof(InvalidGeneric) || type == typeof(Ambiguous))) + { + Assert.ThrowsExactly(() => nativeServices.BuildServiceProvider( + new ServiceProviderOptions { ValidateOnBuild = true })); + var error = Assert.ThrowsExactly(() => Build(services, kind)); + if (type == typeof(Ambiguous)) StringAssert.Contains(error.ToString(), "ambiguous"); + else Assert.IsInstanceOfType(error.InnerExceptions[0].InnerException); + Assert.AreEqual(0, counts.Parts); + Assert.AreEqual(0, nativeCounts.Parts); + Assert.AreEqual(0, counts.KeyProbeRequests); + continue; + } using var provider = Build(services, kind); using var scope = provider.CreateScope(); if (type == typeof(InvalidGeneric)) @@ -131,7 +143,7 @@ IServiceCollection Configure(Counts tracker) [DataRow("snapshot", true)] [DataRow("diagnostics", false)] [DataRow("diagnostics", true)] - public void ActualKeyedDependenciesStillRequireTheirAvailabilityProbe(string kind, bool exposeKeyedProbe) + public void NativeOriginalKeyedDependenciesNeedNoFactoryAvailabilityProbe(string kind, bool exposeKeyedProbe) { foreach (var type in new[] { typeof(Explicit), typeof(Inherited) }) { @@ -145,15 +157,8 @@ public void ActualKeyedDependenciesStillRequireTheirAvailabilityProbe(string kin 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); + Assert.AreEqual(type == typeof(Explicit) ? "explicit" : "inherited", Resolve(scope.ServiceProvider, "blue").Selected); + Assert.AreEqual(0, counts.KeyProbeRequests, "Native type activation uses the provider's own call sites."); } } diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs index 7795110..0d75a55 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.KeyedActivation.Tests.cs @@ -26,7 +26,7 @@ public void TypeActivationPreservesContextLifetimeAndDisposal(ServiceLifetime li using (var scope = native.CreateScope()) Check(scope.ServiceProvider.GetRequiredKeyedService("blue"), "blue", 0); for (var i = 0; i < layers; i++) services.Decorate(); - if (diagnostic && wildcard && layers == 0) + if (diagnostic && wildcard) { // Native build validation inspects inherited dependencies under AnyKey. var native = Assert.ThrowsExactly(() => services.BuildServiceProvider( @@ -277,6 +277,10 @@ public async Task KeyAwareAsyncLayersRemainOwnedExactlyOnce(ServiceLifetime life { var services = Services(); ((IServiceCollection)services).Add(ServiceDescriptor.DescribeKeyed(typeof(IAsyncWork), KeyedService.AnyKey, typeof(AsyncWork), lifetime)); + // Supply a wildcard dependency for native startup validation. Concrete + // requests still select the exact blue/red dependencies in Services(). + services.AddKeyedSingleton(KeyedService.AnyKey, new Part("fallback")); + using (var native = services.BuildServiceProvider(new ServiceProviderOptions { ValidateOnBuild = diagnostic })) { } services.Decorate(); services.Decorate(); var provider = Build(services, diagnostic); var scope = provider.CreateAsyncScope(); diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.DecoratorMetadata.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.DecoratorMetadata.Tests.cs index f266817..6180656 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.DecoratorMetadata.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.DecoratorMetadata.Tests.cs @@ -21,7 +21,7 @@ public void MetadataMutationCannotExposePrivateLayersOrChangeDisposal(bool keyed services.Decorate(); services.Decorate(); var privateLayers = services.Where(d => d.ServiceType != typeof(Work)).ToArray(); - Assert.HasCount(2, privateLayers); + Assert.HasCount(3, privateLayers); // Native original, scoped holder and preceding decorator. using var provider = ServiceProviderFactory.CreateServiceProvider(services, new ExtendedServiceProviderOptions { ValidateOnBuild = diagnostics, @@ -42,9 +42,17 @@ public void MetadataMutationCannotExposePrivateLayersOrChangeDisposal(bool keyed typedKeys.Add(layer.ServiceKey!); lifetimes.Add(layer.ServiceType, ServiceLifetime.Singleton, layer.ServiceKey); Assert.IsFalse(provider.IsServiceRegistered(layer.ServiceType)); - Assert.IsFalse(provider.IsKeyedServiceRegistered(layer.ServiceKey!)); - Assert.IsTrue(provider.IsKeyedScopedServiceRegistered(layer.ServiceType, layer.ServiceKey!)); - Assert.IsFalse(provider.IsKeyedSingletonServiceRegistered(layer.ServiceType, layer.ServiceKey!)); + if (layer.ServiceKey == null) + { + Assert.AreEqual(layer.Lifetime == ServiceLifetime.Scoped, provider.IsScopedServiceRegistered(layer.ServiceType)); + Assert.IsFalse(provider.IsSingletonServiceRegistered(layer.ServiceType)); + } + else + { + Assert.AreEqual(Equals(layer.ServiceKey, key), provider.IsKeyedServiceRegistered(layer.ServiceKey)); + Assert.AreEqual(layer.Lifetime == ServiceLifetime.Scoped, provider.IsKeyedScopedServiceRegistered(layer.ServiceType, layer.ServiceKey)); + Assert.IsFalse(provider.IsKeyedSingletonServiceRegistered(layer.ServiceType, layer.ServiceKey)); + } } Assert.IsTrue(provider.IsServiceRegistered()); if (keyed) @@ -78,7 +86,7 @@ public void MetadataMutationCannotChangeDecoratedDiagnosticExemption(bool keyed, var lifetime = singleton ? ServiceLifetime.Singleton : ServiceLifetime.Scoped; services.Add(new ServiceDescriptor(typeof(Consumer), key, typeof(Consumer), lifetime)); services.Decorate(); - var privateLayer = services.Single(d => d.ServiceType != typeof(Consumer) && d.ServiceType != typeof(DisposableDependency)); + var privateLayer = services.First(d => d.ServiceType != typeof(Consumer) && d.ServiceType != typeof(DisposableDependency)); using var provider = ServiceProviderFactory.CreateServiceProvider(services, new ExtendedServiceProviderOptions { ValidateOnBuild = true, diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.GenericLifetimeSnapshot.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.GenericLifetimeSnapshot.Tests.cs index 4a6c9e6..a430ec6 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.GenericLifetimeSnapshot.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.GenericLifetimeSnapshot.Tests.cs @@ -107,7 +107,7 @@ public void GenericDiagnosticExemptionUsesActualKeyAndIgnoresPublicMutation(bool if (decorated) { services.Decorate, ConsumerDecorator>(); - privateLayer = services.Single(d => d.ServiceType != typeof(Consumer<>) + privateLayer = services.First(d => d.ServiceType != typeof(Consumer<>) && d.ServiceType != typeof(Consumer) && d.ServiceType != typeof(DisposableDependency)); } using var provider = ServiceProviderFactory.CreateServiceProvider(services, new ExtendedServiceProviderOptions @@ -125,9 +125,9 @@ public void GenericDiagnosticExemptionUsesActualKeyAndIgnoresPublicMutation(bool if (privateLayer != null) { metadata.Add(privateLayer.ServiceType, unkeyedLifetime, privateLayer.ServiceKey); - AssertLifetime(provider, privateLayer.ServiceType, privateLayer.ServiceKey, keyedLifetime); + AssertLifetime(provider, privateLayer.ServiceType, privateLayer.ServiceKey, privateLayer.Lifetime); Assert.IsFalse(provider.IsServiceRegistered(privateLayer.ServiceType)); - Assert.IsFalse(provider.IsKeyedServiceRegistered(privateLayer.ServiceKey!)); + Assert.IsTrue(provider.IsKeyedServiceRegistered(privateLayer.ServiceKey!)); // The native original retains the public key. } AssertLifetime(provider, typeof(Consumer), "blue", keyedLifetime); if (keyedSingleton) diff --git a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.OpenGenericSnapshot.Tests.cs b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.OpenGenericSnapshot.Tests.cs index 4616df7..898569f 100644 --- a/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.OpenGenericSnapshot.Tests.cs +++ b/src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.OpenGenericSnapshot.Tests.cs @@ -52,7 +52,7 @@ public void OpenSiblingAndDecoratedClosedGenericKeepPrivateLayersHidden(bool dia services.Decorate, RepositoryDecorator>(); services.Decorate, RepositoryDecorator>(); var layers = services.Where(d => d.ServiceType != typeof(IRepository<>) && d.ServiceType != typeof(IRepository)).ToArray(); - Assert.HasCount(2, layers); + Assert.HasCount(3, layers); // Native original, scoped holder and preceding decorator. using var provider = Build(services, diagnostics); Tamper(provider); foreach (var layer in layers) @@ -61,8 +61,9 @@ public void OpenSiblingAndDecoratedClosedGenericKeepPrivateLayersHidden(bool dia provider.GetRequiredService().Add(layer.ServiceKey!); provider.GetRequiredService>>().Add(layer.ServiceKey!); Assert.IsFalse(provider.IsServiceRegistered(layer.ServiceType)); - Assert.IsFalse(provider.IsKeyedServiceRegistered(layer.ServiceKey!)); - Assert.IsTrue(provider.IsKeyedScopedServiceRegistered(layer.ServiceType, layer.ServiceKey!)); + Assert.AreEqual(layer.ServiceKey != null && Equals(layer.ServiceKey, "decorated"), + provider.IsKeyedServiceRegistered(layer.ServiceKey!)); + Assert.AreEqual(layer.Lifetime == ServiceLifetime.Scoped, provider.IsKeyedScopedServiceRegistered(layer.ServiceType, layer.ServiceKey!)); } using var scope = provider.CreateScope(); var outer = (RepositoryDecorator)scope.ServiceProvider.GetRequiredKeyedService>("decorated"); diff --git a/src/Mammoth.Extensions.DependencyInjection/DecorationServiceType.cs b/src/Mammoth.Extensions.DependencyInjection/DecorationServiceType.cs new file mode 100644 index 0000000..e367272 --- /dev/null +++ b/src/Mammoth.Extensions.DependencyInjection/DecorationServiceType.cs @@ -0,0 +1,15 @@ +using System.Reflection; +using System.Runtime.CompilerServices; + +namespace Mammoth.Extensions.DependencyInjection; + +// Object accepts every implementation; reference identity isolates each original +// registration without changing its key or hiding its native constructor call site. +internal sealed class DecorationServiceType() : TypeDelegator(typeof(object)) +{ + public override Type UnderlyingSystemType => this; + public override bool IsAssignableFrom(Type? candidate) => ReferenceEquals(this, candidate) || typeof(object).IsAssignableFrom(candidate); + public override bool Equals(object? value) => ReferenceEquals(this, value); + public override bool Equals(Type? value) => ReferenceEquals(this, value); + public override int GetHashCode() => RuntimeHelpers.GetHashCode(this); +} diff --git a/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs b/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs index 60819bd..8ab089d 100644 --- a/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs +++ b/src/Mammoth.Extensions.DependencyInjection/DetectIncorrectUsageOfTransientDisposables.cs @@ -23,6 +23,13 @@ public static IServiceCollection PatchForResolutionContextTracking(IServiceColle for (int i = 0; i < services.Count; i++) { var descriptor = services[i]; + // The public decorator factory already tracks this layer's context. + // Preserve native planning rather than hiding the original in another factory. + if (descriptor.ServiceType is DecorationServiceType) + { + collection.Add(descriptor); + continue; + } if (!descriptor.IsKeyedService) { @@ -161,6 +168,12 @@ public static (IServiceCollection ServiceCollection, List Ope foreach (var descriptor in containerBuilder) { + // Native originals are checked by their decorator before resolution. + if (descriptor.ServiceType is DecorationServiceType) + { + collection.Add(descriptor); + continue; + } // If the public service type matches an exclusion pattern, skip patching. if (descriptor.Lifetime == ServiceLifetime.Transient && exclusionPatterns?.Any() == true) { @@ -224,6 +237,23 @@ public static (IServiceCollection ServiceCollection, List Ope return (collection, openGenericDisposables); } + internal sealed class NativeDecorationOptions(bool allowSingleton, IEnumerable? exclusions) + { + internal readonly bool AllowSingleton = allowSingleton; + internal readonly string[] Exclusions = exclusions?.ToArray() ?? []; + } + + internal static void CheckNativeDecoration(IServiceProvider provider, ServiceDescriptor original, object? key) + { + var implementation = original.IsKeyedService ? original.KeyedImplementationType : original.ImplementationType; + if (original.Lifetime != ServiceLifetime.Transient || !IsDisposableType(implementation)) return; + var options = provider.GetService(); + if (options == null || !provider.GetRequiredService().GetIsRootScope() || IsResolvedBySingleton(options.AllowSingleton)) return; + var name = original.ServiceType.FullName; + if (name != null && options.Exclusions.Any(pattern => Regex.IsMatch(name, pattern))) return; + ThrowTransientDisposableException(key, original.ServiceType, implementation, isFactory: false); + } + private static ServiceDescriptor CreatePatchedFactoryDescriptor( ServiceDescriptor original, bool allowSingletonToResolveTransientDisposables diff --git a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Decorators.cs b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Decorators.cs index fcd18c5..0bac18b 100644 --- a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Decorators.cs +++ b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Decorators.cs @@ -55,6 +55,38 @@ object CreateDecorator(IServiceProvider provider, object? requestedKey) var keyed = original.IsKeyedService; var instance = keyed ? original.KeyedImplementationInstance : original.ImplementationInstance; if (instance != null) return (_, _) => instance; + var implementation = keyed ? original.KeyedImplementationType : original.ImplementationType; + if (implementation != null) + { + // Keep type originals visible to native graph planning, including rejected + // constructors, while isolating caching without changing the service key. + var nativeType = new DecorationServiceType(); + var nativeLifetime = original.Lifetime == ServiceLifetime.Scoped ? ServiceLifetime.Transient : original.Lifetime; + services.Add(keyed + ? ServiceDescriptor.DescribeKeyed(nativeType, original.ServiceKey, implementation, nativeLifetime) + : ServiceDescriptor.Describe(nativeType, implementation, nativeLifetime)); + object ResolveNative(IServiceProvider provider, object? requestedKey) + { + DetectIncorrectUsageOfTransientDisposables.CheckNativeDecoration(provider, original, requestedKey); + return keyed ? provider.GetRequiredKeyedService(nativeType, requestedKey) + : provider.GetRequiredService(nativeType); + } + if (original.Lifetime != ServiceLifetime.Scoped) return ResolveNative; + // Native IL emits scoped cache types as ldtoken, losing delegated identity. + // Cache a non-disposable holder under a runtime marker instead. Native DI + // owns the transient original exactly once in the holder's owning scope. + var cacheType = typeof(DecorationSlot); + while (services.Any(d => d.ServiceType == cacheType)) + cacheType = typeof(DecorationSlot<>).MakeGenericType(cacheType); + services.Add(keyed + ? ServiceDescriptor.DescribeKeyed(cacheType, original.ServiceKey, + (provider, key) => new ScopedDecoration(ResolveNative(provider, key)), ServiceLifetime.Scoped) + : ServiceDescriptor.Describe(cacheType, + provider => new ScopedDecoration(ResolveNative(provider, null)), ServiceLifetime.Scoped)); + return (provider, key) => ((ScopedDecoration)(keyed + ? provider.GetRequiredKeyedService(cacheType, key) + : provider.GetRequiredService(cacheType))).Instance; + } var identity = new object(); var slotType = typeof(DecorationSlot); if (keyed) @@ -79,15 +111,13 @@ private static object CreateRegisteredInstance(IServiceProvider provider, Servic return descriptor.KeyedImplementationFactory(provider, requestedKey); if (!keyed && descriptor.ImplementationFactory != null) return descriptor.ImplementationFactory(provider); - var implementation = keyed ? descriptor.KeyedImplementationType! : descriptor.ImplementationType!; - // Reactivate an original type descriptor with native DI constructor rules. - // Factory descriptors above retain their own activation policy (including maps - // and preceding decorators). An unkeyed original must not see the private slot key. - return NativeConstructorActivator.CreateInstance(provider, implementation, keyed ? requestedKey : null); + throw new InvalidOperationException("Expected an original implementation factory."); } internal static bool IsDecorationSlot(Type type) => - type.IsGenericType && type.GetGenericTypeDefinition() == typeof(DecorationSlot<>); + type is DecorationServiceType || + (type.IsGenericType && type.GetGenericTypeDefinition() == typeof(DecorationSlot<>)); + private sealed class ScopedDecoration(object instance) { internal object Instance { get; } = instance; } private sealed class DecorationSlot; } diff --git a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs index 65db5cc..1a99ead 100644 --- a/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs +++ b/src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Registration.cs @@ -64,8 +64,10 @@ public static ServiceDescriptor[] GetServiceDescriptors(this IServiceCollection throw new ArgumentNullException(nameof(serviceType)); } + if (IsDecorationSlot(serviceType)) return []; return services.Where(serviceDescriptor => - (isKeyedService == null || serviceDescriptor.IsKeyedService == isKeyedService) + !IsDecorationSlot(serviceDescriptor.ServiceType) + && (isKeyedService == null || serviceDescriptor.IsKeyedService == isKeyedService) && serviceType.IsAssignableFrom(serviceDescriptor.ServiceType)) .ToArray(); } @@ -255,10 +257,11 @@ public static bool IsKeyedTransientServiceRegistered(this IService { // Preserve the null-collection exception previously raised by LINQ's Where. if (source is null) throw new ArgumentNullException(nameof(source)); + if (IsDecorationSlot(serviceType)) return null; for (var i = source.Count - 1; i >= 0; i--) { var descriptor = source[i]; - if (descriptor.IsKeyedService == isKeyedService + if (!IsDecorationSlot(descriptor.ServiceType) && descriptor.IsKeyedService == isKeyedService && serviceType.IsAssignableFrom(descriptor.ServiceType) && (!isKeyedService || Equals(descriptor.ServiceKey, serviceKey))) return descriptor; diff --git a/src/Mammoth.Extensions.DependencyInjection/ServiceProviderFactory.cs b/src/Mammoth.Extensions.DependencyInjection/ServiceProviderFactory.cs index 56433bc..7404d2d 100644 --- a/src/Mammoth.Extensions.DependencyInjection/ServiceProviderFactory.cs +++ b/src/Mammoth.Extensions.DependencyInjection/ServiceProviderFactory.cs @@ -80,6 +80,9 @@ public static ServiceProvider CreateServiceProvider(IServiceCollection container options.DetectIncorrectUsageOfTransientDisposablesExclusionPatterns ); sc = DetectIncorrectUsageOfTransientDisposables.PatchForResolutionContextTracking(patchedSc); + sc.AddSingleton(new DetectIncorrectUsageOfTransientDisposables.NativeDecorationOptions( + options.AllowSingletonToResolveTransientDisposables, + options.DetectIncorrectUsageOfTransientDisposablesExclusionPatterns)); var sp = sc.BuildServiceProvider(options); if (openGenerics.Count > 0 && !options.ThrowOnOpenGenericTransientDisposable) { diff --git a/test-support/NativeResolverCompilation.cs b/test-support/NativeResolverCompilation.cs index 3391981..1fc4854 100644 --- a/test-support/NativeResolverCompilation.cs +++ b/test-support/NativeResolverCompilation.cs @@ -27,6 +27,9 @@ private NativeResolverCompilation(ServiceProvider provider, Type serviceType, ob internal static NativeResolverCompilation Observe(ServiceProvider provider, object? key) => new(provider, typeof(IEnumerable), key); + internal static NativeResolverCompilation ObserveService(ServiceProvider provider, Type type, object? key) => + new(provider, type, key); + internal void WaitForReplacement() { var timer = Stopwatch.StartNew(); From 66fecf1eeddecd01c1ef8f1f779d6749cf88f8e5 Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Fri, 9 Oct 2026 19:17:13 +0200 Subject: [PATCH 4/5] docs: explain decorator design and disposal ownership Document native original activation, private registration identities, scoped holders, factory layers, validation boundaries, and design tradeoffs with executable examples. Make inner-service disposal ownership explicit in the usage guide and packaged skill. Co-authored-by: Codex --- .agents/skills/mammoth-di/SKILL.md | 3 +- .agents/skills/mammoth-di/references/usage.md | 8 +- README.md | 31 +- docs/original-decoration-graph-planning.md | 348 +++++++++++++++++- 4 files changed, 371 insertions(+), 19 deletions(-) diff --git a/.agents/skills/mammoth-di/SKILL.md b/.agents/skills/mammoth-di/SKILL.md index 1e6f081..cbe5f82 100644 --- a/.agents/skills/mammoth-di/SKILL.md +++ b/.agents/skills/mammoth-di/SKILL.md @@ -17,7 +17,8 @@ Imports: `Microsoft.Extensions.DependencyInjection`, `Mammoth.Extensions.Depende ## Choose registration and resolution deliberately -- Decorate with `Decorate()` after the intended registration. It wraps only the last exact service registration, preserving its key/lifetime/order; repeat calls nest outer layers. Container-created inner services and decorators are disposed independently. Never forward decorator disposal to the injected inner; caller-supplied instances stay caller-owned. This closed generic API does not register open-generic decorators. Original type registrations retain native graph validation before activation, including rejected constructor candidates and enumerable members; `ValidateOnBuild` may reject them at startup. Factory originals and non-empty maps remain opaque to that inspection. +- Decorate with `Decorate()` after the intended registration. It wraps only the last exact service registration, preserving its key/lifetime/order; repeat calls nest outer layers. This closed generic API does not register open-generic decorators. Original type registrations retain native graph validation before activation, including rejected constructor candidates and enumerable members; `ValidateOnBuild` may reject them at startup. Factory originals and non-empty maps remain opaque to that inspection. +- **Decorators must not dispose their injected inner service**, through either `Dispose()` or `DisposeAsync()`. DI owns each container-created original and decorator independently. Decorators release only resources they create and own themselves; instance registrations remain caller-owned. Use the [ownership recipe](references/usage.md#keyed-decorators-and-caller-owned-instances) to verify scoped layers and caller ownership. - Map **parameter names** using `Parameter.ForKey("store").Eq("primary")` or `Dependency.OnValue("label", "batch")`. `Eq` accepts a string key; other native keys can use a suitable attribute constant or an explicit factory. Values must be assignable; maps are not global registrations. Unused names are ignored, so verify names against the constructor. - Non-empty DependsOn maps select at resolution: one preferred `[ActivatorUtilitiesConstructor]` 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 services; optional defaults apply only to unregistered dependencies. `[FromKeyedServices]` inherits the current key; `[FromKeyedServices(null)]` performs unkeyed lookup. `[ServiceKey]` only injects a non-null current key. For both attributes on one untouched parameter, metadata order selects the first effective non-null binding; named maps override both. Rejected constructors' dependencies are not created. Empty maps delegate to native DI. Mapped factories need keyed resolution plus ordinary/keyed service probes. Native original type registrations behind decorators need no separate keyed availability probe; contextual decorator constructors retain their probes. - Build with `ServiceProviderFactory.CreateServiceProvider(services, options)` when using Mammoth provider queries/discovery. For a host, use its `UseServiceProviderFactory` hook rather than constructing a second provider. Native BuildServiceProvider alone does not create Mammoth's authoritative metadata. diff --git a/.agents/skills/mammoth-di/references/usage.md b/.agents/skills/mammoth-di/references/usage.md index 83b4318..f736eb9 100644 --- a/.agents/skills/mammoth-di/references/usage.md +++ b/.agents/skills/mammoth-di/references/usage.md @@ -151,7 +151,11 @@ public sealed class Scanned(string label) { public string Label { get; } = label ## Keyed decorators and caller-owned instances -Decorate the intended keyed registration immediately, before adding another registration of that service type. Native original type graphs are validated before activation; enabling `ValidateOnBuild` can move graph errors to startup. Factory originals and non-empty maps retain their own activation policies. Each wrapper releases only its own resources. A caller-owned singleton is explicitly disposed by the caller after the provider releases its wrapper. +> **Decorators must not dispose their injected inner service.** This includes both `Dispose()` and `DisposeAsync()`. DI owns container-created originals and every decorator independently. A decorator releases only resources it creates and owns itself; it never forwards disposal to `Inner`. + +Decorate the intended keyed registration immediately, before adding another registration of that service type. Native original type graphs are validated before activation; enabling `ValidateOnBuild` can move graph errors to startup. Factory originals and non-empty maps retain their own activation policies. A caller-supplied singleton registered with the instance overload remains caller-owned and is explicitly disposed by the caller after the provider releases its wrapper. A factory returning that same instance would instead make DI track its disposal. + +The recipe below verifies exactly-once disposal for each scoped layer and preservation of caller ownership. Forwarding `Inner.Dispose()` from `StoreDecorator` would break these assertions. For an async decorator, release only its own async resources in `DisposeAsync()` and use async scope/provider disposal when any layer is async-only. The [detailed design](../../../../docs/original-decoration-graph-planning.md) includes a complete async scope example and explains the native original registration and scoped holder. ```csharp using Microsoft.Extensions.DependencyInjection; @@ -195,6 +199,6 @@ public sealed class StoreDecorator(IStore inner) : IStore { public IStore Inner { get; } = inner; public int DisposeCount { get; private set; } - public void Dispose() => DisposeCount++; // do not dispose Inner + public void Dispose() => DisposeCount++; // Own cleanup only; never dispose Inner. } ``` diff --git a/README.md b/README.md index 11d250a..e98856e 100644 --- a/README.md +++ b/README.md @@ -33,7 +33,11 @@ Import `Microsoft.Extensions.DependencyInjection` and `Mammoth.Extensions.Depend ### Decorator -Use the `Decorator` extension to wrap an existing service with a new implementation without altering the original. +Use `Decorate()` to wrap an existing service with a decorator. + +> **Decorators must not dispose their injected inner service.** Never forward `Dispose()` or `DisposeAsync()` to the inner service. DI disposes every container-created original and decorator independently; instances supplied through an instance registration remain caller-owned. A decorator releases only resources it creates and owns itself. + +See [decorator disposal and ownership](#decorator-disposal-and-ownership) for a safe implementation, and [the detailed design](docs/original-decoration-graph-planning.md) for native activation, scoped holders, factory layers, and the reasons for this registration strategy. Both interface-based and class-based services can be decorated. The following examples demonstrate how to decorate services. @@ -125,7 +129,30 @@ using var scope = provider.CreateScope(); var decorated = scope.ServiceProvider.GetRequiredKeyedService("one"); ``` -Container-created inner services and every decorator are disposed independently. A disposable decorator must dispose its own resources without forwarding disposal to its injected inner service. Caller-supplied instances remain caller-owned. See the [complete ownership example](.agents/skills/mammoth-di/references/usage.md#keyed-decorators-and-caller-owned-instances). +#### Decorator disposal and ownership + +**Decorators must not dispose their injected inner service**, including through `DisposeAsync()`. DI already owns container-created inner services and every decorator. Forwarding disposal can dispose an inner layer twice; for caller-supplied instances, it violates caller ownership. A decorator that owns no resources does not need to implement `IDisposable` or `IAsyncDisposable` merely because its inner service does. + +This decorator disposes its own buffer only: + +```csharp +public interface IBufferedService { void Run(); } + +public sealed class BufferedServiceDecorator(IBufferedService inner) + : IBufferedService, IDisposable, IAsyncDisposable +{ + private readonly MemoryStream _buffer = new(); // Owned by this decorator. + public void Run() { _buffer.WriteByte(1); inner.Run(); } + public void Dispose() => _buffer.Dispose(); // Never dispose inner here. + public ValueTask DisposeAsync() + { + Dispose(); // Release only the owned buffer; do not call inner.DisposeAsync(). + return default; + } +} +``` + +Dispose the owning scope/provider to release container-owned layers. Use async scope/provider disposal for async-only resources. For `AddSingleton(existingInstance)` or its keyed instance overload, the caller remains responsible for `existingInstance`; a factory returning an object gives DI ownership of its result. See the [complete keyed ownership example](.agents/skills/mammoth-di/references/usage.md#keyed-decorators-and-caller-owned-instances) and the [design's runnable scoped example](docs/original-decoration-graph-planning.md#the-ownership-rule). ### DependsOn (requires Keyed Services support) diff --git a/docs/original-decoration-graph-planning.md b/docs/original-decoration-graph-planning.md index c36eab0..1bf96bc 100644 --- a/docs/original-decoration-graph-planning.md +++ b/docs/original-decoration-graph-planning.md @@ -1,27 +1,347 @@ -# Native graph planning for decorated originals +# Decoration: native activation, caching, and disposal ownership -Issue #121 regressed from 0.7.1 when #37 moved original services behind private factories to fix disposal ownership. Factories hid constructor graphs: invalid rejected candidates could be skipped, and selected graphs could activate an earlier dependency factory before failing. +This document explains [PR #124](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/pull/124), which fixes [issue #121](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/121). The design restores native Microsoft DI constructor planning for decorated original **implementation-type registrations**, while retaining independent disposal ownership and registration isolation. These changes belong to the release after 0.8.0; they are not guarantees of the published 0.8.0 package. -## Registration design +For application setup, see the [usage guide](../README.md#decorator) and [keyed ownership recipe](../.agents/skills/mammoth-di/references/usage.md#keyed-decorators-and-caller-owned-instances). The implementation is in [ServiceCollectionExtensions.Decorators.cs](../src/Mammoth.Extensions.DependencyInjection/ServiceCollectionExtensions.Decorators.cs) and [DecorationServiceType.cs](../src/Mammoth.Extensions.DependencyInjection/DecorationServiceType.cs). -`RegisterInnerLayer` keeps original implementation types as native DI type registrations. Each receives a private `DecorationServiceType`, a public `TypeDelegator` over `object` with reference identity and explicit object assignability. `UnderlyingSystemType` retains that private identity, so normal object registrations compare unequal. The original public service key is preserved, including unkeyed/null-key context, rather than replacing it with a private key. +## The ownership rule -Native DI plans constructors and dependency call sites before constructing the original. It owns that original and the public decorator independently. Caller-supplied instances remain caller-owned. Factory/map originals and preceding decorators retain the existing private factory slots and activation policy. No extra provider is built for runtime graph checking, and no application factory is used as a validation probe. +> **Decorators must not dispose their injected inner service.** This applies to both `Dispose()` and `DisposeAsync()`. -## Scoped compiled caching +DI tracks each container-created original and each decorator independently. A decorator disposes only resources that it creates and owns itself. Injecting an inner service does not transfer its ownership to the decorator. This follows [Microsoft's DI ownership guidance](https://learn.microsoft.com/en-us/dotnet/core/extensions/dependency-injection/guidelines#general-idisposable-guidelines). -The [native DI 10 IL emitter](https://github.com/dotnet/runtime/blob/v10.0.0/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ILEmit/ILEmitResolverBuilder.cs#L248-L256) reconstructs scoped cache keys using a runtime type token. A delegated identity cannot survive that operation. A scoped original therefore uses a transient native type registration plus a scoped, non-disposable holder under a normal private runtime marker type. Native DI creates/captures the inner in that scope; the holder caches its reference without capturing disposal a second time. Singleton and transient originals keep their native lifetimes. Public service and decorator lifetimes are unchanged. +| Registration/layer | Owner responsible for disposal | +| --- | --- | +| Original registered by implementation type | DI's owning scope/provider | +| Original returned from an implementation factory | DI's owning scope/provider | +| Every container-created decorator, including intermediate layers | DI's owning scope/provider | +| Original supplied through an instance overload | The caller | +| Private scoped holder | Non-disposable; it stores a reference only | +| Resource created inside a decorator, such as its own buffer | That decorator | -The native original and scoped holder are both hidden from public Mammoth discovery. Keys remain public keys, so a key-only query can still report the original registration's key. Private descriptor lifetimes are implementation details; the public service lifetime stays authoritative. +These implementations are incorrect for an injected inner service: -## Diagnostics and validation timing +```csharp +// Incorrect: DI or the caller already owns _inner. +public void Dispose() => _inner.Dispose(); +public ValueTask DisposeAsync() => _inner.DisposeAsync(); +``` -Diagnostics preserve the native type descriptor and track resolution through the public decorator factory. A small per-provider options service lets that factory check transient disposable originals before creation, with the existing exclusion and singleton-ancestor rules. The shared caller collection is never reconfigured per provider. +Forwarding disposal can dispose an original or intermediate decorator twice. With a supplied instance, it can dispose an object the caller still intends to use. A decorator that owns no resources can omit `IDisposable`/`IAsyncDisposable`, even if its inner implements them. -`ValidateOnBuild` can now reject a decorated original at startup, just as it rejects the corresponding undecorated native type. This includes ambiguity, invalid generic constraints and `AnyKey` inherited dependencies that have no wildcard binding at validation time. Use a valid wildcard dependency or concrete registrations when startup validation needs one. Disabling validation for diagnostics remains unsupported. +This complete example creates two scoped decorators. Each owns its buffer; neither disposes `Inner`. Async scope disposal cleans up both decorators and the original exactly once. -Factories, non-empty dependency maps and decorator constructors are opaque to native graph planning. This restores native original-type behavior; it does not promise inspection through arbitrary application factories or change DependsOn's separate selection policy. + +```csharp +using Microsoft.Extensions.DependencyInjection; +using Mammoth.Extensions.DependencyInjection; -## Regression evidence +IServiceCollection services = new ServiceCollection(); +services.AddScoped(); +services.Decorate(); +services.Decorate(); -Differential tests compare native and decorated registrations under the same validation settings, require zero factory/constructor calls on graph failures, and cover rejected/selected constructors, open/closed generics, enumerable ordering, cycles, keys, lifetimes, repeated layers and provider isolation. Native-only test instrumentation observes compiled accessor replacement for originals and scoped holders, then checks 100 repeated resolutions, independent caches, ordinary object/implementation registrations and exactly-once disposal across all supported test targets. Instrumentation is never included in the production library. +await using var provider = services.BuildServiceProvider( + new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }); +var scope = provider.CreateAsyncScope(); +var outer = (BufferedWorker)scope.ServiceProvider.GetRequiredService(); +var middle = (BufferedWorker)outer.Inner; +var original = (Worker)middle.Inner; +outer.Run(); +if (!ReferenceEquals(outer, scope.ServiceProvider.GetRequiredService())) + throw new Exception("The scoped decorator must be reused."); +await scope.DisposeAsync(); +if (outer.DisposeCount != 1 || middle.DisposeCount != 1 || original.DisposeCount != 1) + throw new Exception("Every container-owned layer must be disposed once."); +Console.WriteLine("PASS: scoped reuse and independent async disposal"); + +public interface IWorker { void Run(); } +public sealed class Worker : IWorker, IDisposable +{ + public int DisposeCount { get; private set; } + public void Run() { } + public void Dispose() => DisposeCount++; +} +public sealed class BufferedWorker(IWorker inner) : IWorker, IDisposable, IAsyncDisposable +{ + private readonly MemoryStream _buffer = new(); // Created and owned here. + public IWorker Inner { get; } = inner; + public int DisposeCount { get; private set; } + public void Run() { _buffer.WriteByte(1); Inner.Run(); } + public void Dispose() + { + _buffer.Dispose(); // Release this decorator's own resource only. + DisposeCount++; + // Do not call Inner.Dispose(). + } + public ValueTask DisposeAsync() + { + Dispose(); // Only this decorator's buffer; no forwarding to Inner. + return default; + } +} +``` + +For caller ownership, use `services.AddSingleton(existingWorker)` or a keyed instance overload. The provider disposes the decorators; the caller disposes `existingWorker` after it no longer needs it. A factory registration such as `services.AddSingleton(_ => existingWorker)` gives DI ownership of its returned object. Do not use that factory form when the supplied object must remain caller-owned. + +## Registration and resolution flow + +`Decorate()` finds the **last exact registration of `TService`**, preserving its public lifetime, key, and position in the collection. It registers the inner layer privately and replaces the public descriptor with a decorator factory. If several keys exist, it decorates the last registration, whichever key it has; this API has no separate key-selection argument. + +Repeated calls add outer layers. Using the types above: + +```csharp +services.AddScoped(); +services.Decorate(); // First, innermost decorator. +services.Decorate(); // Second, outermost decorator. +``` + +The application receives `BufferedWorker(outer) -> BufferedWorker(inner) -> Worker`. First resolution follows this registration path: + +```mermaid +flowchart TD + Public["Public IWorker: scoped outer decorator factory"] --> Previous["Private scoped factory: preceding decorator"] + Previous --> Holder["Private scoped holder"] + Holder --> Original["Native transient descriptor: Worker"] + Original --> Dependencies["Native dependency call sites"] +``` + +Construction returns the original to the first decorator, then that decorator to the outer one. The holder is internal infrastructure; the application decorator receives the original object. Later resolutions reuse the scoped outer decorator; a new scope gets a separate chain. + +Unkeyed decorators use `ActivatorUtilities.CreateInstance(provider, inner)`. Keyed decorators use Mammoth's contextual `ConstructorActivator.CreateKeyed(provider, typeof(TDecorator), requestedKey, inner)`. Both receive the inner explicitly. Native DI constructs the original implementation type. + +The API supports closed service types, including closed generics. It does not register open-generic decorator definitions. + +## Why original constructors must remain native + +The #37 disposal fix moved originals into private factory registrations. DI could then independently track each returned original and intermediate decorator. However, the same change converted native implementation-type registrations into opaque factories and caused #121, a regression from 0.7.1 present in 0.8.0. + +Native DI plans constructor/dependency call sites before activation. A factory exposes a delegate, not its internal constructor graph. Checking whether dependencies are registered cannot reproduce full planning: a registered dependency may itself have a missing nested dependency, invalid generic constraints, an invalid enumerable member, or a cycle. + +| Invalid graph | Native DI | Previous private activation path | +| --- | --- | --- | +| Selected constructor with an earlier factory and later invalid nested dependency | Rejects before the factory runs | Can run the earlier factory before failing | +| Rejected constructor candidate containing an invalid nested graph | Can fail while planning that candidate | Can skip the candidate and succeed through another constructor | + +This complete example checks the selected-graph case. Neither the original nor its decorator should be constructed, and the `Part` factory must not run. + + +```csharp +using Microsoft.Extensions.DependencyInjection; +using Mammoth.Extensions.DependencyInjection; + +foreach (var decorated in new[] { false, true }) +{ + int factoryCalls = 0; + IServiceCollection services = new ServiceCollection(); + services.AddTransient(_ => { factoryCalls++; return new Part(); }); + services.AddTransient(); + services.AddScoped(); + if (decorated) services.Decorate(); + using var provider = services.BuildServiceProvider(); + using var scope = provider.CreateScope(); + bool failed = false; + try { scope.ServiceProvider.GetRequiredService(); } + catch (InvalidOperationException error) + { + if (!error.Message.Contains(nameof(MissingOther))) throw; + failed = true; + } + if (!failed || factoryCalls != 0) + throw new Exception("Invalid graphs must fail before dependency activation."); +} +Console.WriteLine("PASS: native and decorated graphs fail before activation"); + +public interface IWorker { } +public interface IBroken { } +public sealed class Part { } +public sealed class MissingOther { } +public sealed class Broken : IBroken +{ + public Broken(MissingOther missing) { } +} +public sealed class Worker : IWorker +{ + public Worker(Part part, IBroken broken) { } +} +public sealed class Forwarder(IWorker inner) : IWorker +{ + public IWorker Inner { get; } = inner; +} +``` + +For a rejected candidate, consider these constructors on an original registered under `IWorker`: + +```csharp +public Worker(Part part, Other other) { } +public Worker(IBroken broken, Missing missing) { } +``` + +Even when `Missing` is unregistered, native DI can encounter the invalid `IBroken` graph while examining the second candidate. An availability-only activator can skip that candidate without examining the nested graph. Differential tests compare native behavior directly rather than assuming these policies are equivalent. + +## Native implementation types and isolated identities + +The design retains the real `ImplementationType` on a native `ServiceDescriptor`. It changes the private service identity while preserving the original key and constructor context. + +| Original registration | Private representation | Caching | +| --- | --- | --- | +| Transient implementation type | Native type descriptor under a unique delegated type | Native transient behavior | +| Singleton implementation type | Native type descriptor under a unique delegated type | Native singleton cache | +| Scoped implementation type | Native transient descriptor plus scoped holder | Original cached per scope and requested key | +| Factory or non-empty DependsOn map | Private factory slot | Native cache for the original lifetime | +| Preceding decorator factory | Private factory slot | Native cache for that layer's lifetime | +| Supplied instance | Captured reference | Existing caller-owned object | + +Re-registering originals under their public implementation type would not isolate them. An existing `Worker` binding could be shadowed; two `IWorker -> Worker` registrations with the same key could resolve the same last `Worker` binding even if their intended lifetimes differed. A new private key would isolate registrations but change the key observed by `[ServiceKey]` and inherited-key dependencies. + +`DecorationServiceType` instead provides a reference-identity `TypeDelegator` over `object`: + +```csharp +internal sealed class DecorationServiceType() : TypeDelegator(typeof(object)) +{ + public override Type UnderlyingSystemType => this; + public override bool IsAssignableFrom(Type? candidate) => + ReferenceEquals(this, candidate) || typeof(object).IsAssignableFrom(candidate); + public override bool Equals(object? value) => ReferenceEquals(this, value); + public override bool Equals(Type? value) => ReferenceEquals(this, value); + public override int GetHashCode() => RuntimeHelpers.GetHashCode(this); +} +``` + +Each original gets a new identity. It accepts the original implementation as assignable, while normal `object` registrations remain distinct. It is a private `Type` object, not a generated implementation class or application-visible wrapper; production code does not emit replacement implementation types. + +The simplified native registration is: + +```csharp +var privateType = new DecorationServiceType(); +services.Add(ServiceDescriptor.Describe( + privateType, original.ImplementationType!, nativeLifetime)); +``` + +The keyed path uses `DescribeKeyed(privateType, original.ServiceKey, implementationType, nativeLifetime)` and resolves with the actual requested key. Native DI plans the original's constructors/dependencies and owns its instance independently of the outer factory result. + +## Why scoped originals need a holder in this design + +Ordinary scoped registrations already have native caching. The holder is needed specifically because of the delegated private type identity. + +The [DI 10 IL emitter](https://github.com/dotnet/runtime/blob/v10.0.0/src/libraries/Microsoft.Extensions.DependencyInjection/src/ServiceLookup/ILEmit/ILEmitResolverBuilder.cs#L248-L256) reconstructs scoped cache keys with `ldtoken` and `Type.GetTypeFromHandle`. That operation expects a runtime type identity and cannot preserve the delegated identity. Direct scoped caching under that alias cannot be relied on after native resolver compilation. + +The design separates construction from scoped caching: + +1. The original has a **native transient type descriptor** under its delegated identity. DI plans it, creates it, and captures it for disposal. +2. A **scoped factory under a normal runtime marker type** creates a non-disposable holder containing the original. +3. The decorator resolves the holder and receives its `Instance`. + +Simplified internal code, omitting unique marker allocation, keyed handling, and diagnostics: + +```csharp +services.Add(ServiceDescriptor.Describe( + privateType, typeof(Worker), ServiceLifetime.Transient)); +services.Add(ServiceDescriptor.Describe( + holderType, + sp => new ScopedDecoration(sp.GetRequiredService(privateType)), + ServiceLifetime.Scoped)); + +// Inside the decorator factory: +var inner = ((ScopedDecoration)provider.GetRequiredService(holderType)).Instance; + +private sealed class ScopedDecoration(object instance) +{ + internal object Instance { get; } = instance; +} +``` + +The runtime marker is unique for the registration; native keyed caching distinguishes requested keys. The holder has no custom cache, lock, `Dispose`, or `DisposeAsync`. DI already captures the original through its native transient descriptor. Returning that original directly from another scoped factory would capture the same disposable again; returning a non-disposable holder avoids the second capture. + +Successful original creation is cached independently of decorator construction. If a decorator constructor subsequently throws, the scope still owns the original and its holder. Already-created layers remain owned even when resolution fails. + +The cost is one holder allocation per resolved original registration and requested key per scope, plus its descriptor. Public service/decorator lifetimes remain scoped; the private original descriptor is transient. Private descriptor lifetimes are implementation details. Transient and singleton type originals do not need this holder. + +The holder factory is itself opaque to graph planning; the separate native type descriptor exposes the original graph. Scope validation, compiled cache identity, scope isolation, and disposal require explicit verification. The holder is a workaround for this isolation design, not a general DI requirement. + +## Factories, repeated layers, and supplied instances + +An application factory already determines its own activation policy. Its constructor graph cannot be recovered from the delegate. The design retains a private factory registration so DI can cache and track its returned object. Non-empty DependsOn maps and preceding decorator factories follow this path. + +Factory slots use private runtime marker types and keys. The implementation resolves these slots through the `Type` overload, without casting the returned object to the marker. Keyed slots use an isolated marker with native requested-key caching. Unkeyed slots use a private identity key without passing it into the original unkeyed activation policy. + +On the next `Decorate` call, the preceding public decorator factory becomes a private layer. Each successful layer returns through its own registration and is captured independently. An instance original is instead captured directly, preserving caller ownership; putting it behind a factory returning that instance would transfer disposal ownership to DI. + +## Keys and independent scopes + +This complete example decorates each key immediately after registration. It verifies original/decorator key context, scoped reuse, isolation between keys, and isolation between scopes. + + +```csharp +using Microsoft.Extensions.DependencyInjection; +using Mammoth.Extensions.DependencyInjection; + +IServiceCollection services = new ServiceCollection(); +foreach (var key in new[] { "blue", "red" }) +{ + services.AddKeyedScoped(key); + services.Decorate(); +} +using var provider = services.BuildServiceProvider( + new ServiceProviderOptions { ValidateOnBuild = true, ValidateScopes = true }); +using var scope = provider.CreateScope(); +var blue = (KeyedDecorator)scope.ServiceProvider.GetRequiredKeyedService("blue"); +var red = (KeyedDecorator)scope.ServiceProvider.GetRequiredKeyedService("red"); +if (blue.Key != "blue" || blue.Inner.Key != "blue" || red.Key != "red" + || ReferenceEquals(blue.Inner, red.Inner) + || !ReferenceEquals(blue, scope.ServiceProvider.GetRequiredKeyedService("blue"))) + throw new Exception("Key context or scoped identity was lost."); +using var otherScope = provider.CreateScope(); +var otherBlue = (KeyedDecorator)otherScope.ServiceProvider.GetRequiredKeyedService("blue"); +if (ReferenceEquals(blue.Inner, otherBlue.Inner)) + throw new Exception("Original instances must not leak between scopes."); +Console.WriteLine("PASS: public keys and independent scoped originals"); + +public interface IKeyedWorker { string Key { get; } } +public sealed class KeyedWorker([ServiceKey] string key) : IKeyedWorker +{ + public string Key { get; } = key; +} +public sealed class KeyedDecorator(IKeyedWorker inner, [ServiceKey] string key) : IKeyedWorker +{ + public IKeyedWorker Inner { get; } = inner; + public string Key { get; } = key; +} +``` + +For `KeyedService.AnyKey`, resolution uses the concrete requested key. Native `ValidateOnBuild` may still reject an inherited-key dependency while validating the wildcard registration itself. Use a valid wildcard dependency or concrete registrations when startup validation requires one. Decoration does not redefine those native rules. + +## Diagnostics, discovery, and validation boundaries + +Private originals, holders, and factory slots are filtered from Mammoth's public discovery and registration queries. Public service keys and lifetimes remain authoritative. Native DI can construct and own private layers without Mammoth discovery presenting them as extra application services. + +Transient-disposable diagnostics keep the native original type descriptor visible. The public decorator resolution tracks context; a small immutable per-provider options service applies existing exclusion and singleton-ancestor rules before original construction. This check uses the original public registration's lifetime, not the holder's transient construction descriptor. Provider-specific options do not rewrite the shared caller collection. + +`ValidateOnBuild` can now reject a decorated original at startup, as it rejects the corresponding native graph. Otherwise, native planning occurs when the original is resolved. Factory bodies, non-empty DependsOn maps, and decorator constructors remain opaque to native graph inspection; their activation policies are unchanged. Diagnostics still require `ValidateOnBuild = true` and are not comprehensive leak detection. + +No additional provider is built for runtime graph probing. Application factories are not executed to test availability, and native constructor planning is not duplicated in a shadow graph. The existing diagnostics startup-validation path is a separate concern. + +## Why not restore the earlier implementation unchanged? + +Before #37, originals whose service type differed from their implementation type were re-registered under the real implementation type, retaining their key/lifetime. That path kept native constructor planning and scoped caching without a delegated identity or holder. The factory rewrite was broader than necessary for those ordinary cases. + +However, the earlier code invoked original factories and self-type originals inside decorator factories. DI saw the outer result and did not independently own those originals or intermediate decorators. Public implementation-type registrations could shadow unrelated bindings or make independent decorated bindings resolve the same last original. + +Local probes using exact earlier code and matched DI 10.0.12 confirmed both sides: + +- The earlier native interface-type path and this proposal matched native failure-before-activation behavior in all four keyed/unkeyed constructor cases checked. The pre-fix factory implementation either succeeded incorrectly or ran a dependency factory first. +- Twenty-eight ownership/binding outcomes confirmed native interface-type original disposal, but also reproduced undisposed factory/self-type originals and intermediate decorators, unrelated binding collisions, and scoped/singleton bindings sharing an original. + +A constrained alternative could keep native implementation-type registrations for compatible cases, retain DI-owned factories for factory-created layers, and reject unsupported collisions or self-type cases. It would change the supported registration/discovery contract and is not a verified drop-in replacement. The delegated identity and holder aim to preserve the broader supported cases through additional internal plumbing. Passing regression tests establishes covered behavior, not that this is the only possible architecture. + +## Verification and maintenance + +The tests compare native and decorated behavior under identical registrations and validation settings: + +- [Graph planning](../src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.GraphPlanning.Tests.cs): rejected/selected constructors, nested generics, invalid enumerable members, provider isolation, valid controls, and zero activation on graph errors. +- [Native identity](../src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.NativeIdentity.Tests.cs): observes actual compiled accessor replacement for originals/holders before 100 repeated resolutions; verifies caches, public object/implementation bindings, keys, scopes, and exactly-once disposal. +- [Disposal](../src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.Disposal.Tests.cs): factory/self-type originals, intermediate layers, caller ownership, async disposal, failing decorators, and unrelated bindings. +- [Registration order](../src/Mammoth.Extensions.DependencyInjection.Tests/ServiceCollectionExtensions.Decorators.RegistrationOrder.Tests.cs): repeated descriptor occurrences, order, lifetime, and ownership. +- [Validation](../src/Mammoth.Extensions.DependencyInjection.Tests/ServiceProviderFactory.Validation.Tests.cs): native startup validation and diagnostic options. + +These tests cover net472/net8.0/net9.0/net10.0 validation targets. Compiled-accessor observation is test-only instrumentation excluded from the production library. When changing DI versions or this design, recheck compiled cache behavior and disposal ownership; a cold provider alone does not establish cache identity safety. From a0165ec66f54f3b88ef65f94370ce07734c1bbbb Mon Sep 17 00:00:00 2001 From: Alessandro Giorgetti Date: Fri, 9 Oct 2026 19:39:26 +0200 Subject: [PATCH 5/5] docs: rename decorator architecture reference Co-authored-by: Codex --- .agents/skills/mammoth-di/references/usage.md | 2 +- README.md | 6 +++--- ...coration-graph-planning.md => decorator-architecture.md} | 2 +- docs/keyed-built-in-registration-probing.md | 2 +- 4 files changed, 6 insertions(+), 6 deletions(-) rename docs/{original-decoration-graph-planning.md => decorator-architecture.md} (98%) diff --git a/.agents/skills/mammoth-di/references/usage.md b/.agents/skills/mammoth-di/references/usage.md index f736eb9..f87865a 100644 --- a/.agents/skills/mammoth-di/references/usage.md +++ b/.agents/skills/mammoth-di/references/usage.md @@ -155,7 +155,7 @@ public sealed class Scanned(string label) { public string Label { get; } = label Decorate the intended keyed registration immediately, before adding another registration of that service type. Native original type graphs are validated before activation; enabling `ValidateOnBuild` can move graph errors to startup. Factory originals and non-empty maps retain their own activation policies. A caller-supplied singleton registered with the instance overload remains caller-owned and is explicitly disposed by the caller after the provider releases its wrapper. A factory returning that same instance would instead make DI track its disposal. -The recipe below verifies exactly-once disposal for each scoped layer and preservation of caller ownership. Forwarding `Inner.Dispose()` from `StoreDecorator` would break these assertions. For an async decorator, release only its own async resources in `DisposeAsync()` and use async scope/provider disposal when any layer is async-only. The [detailed design](../../../../docs/original-decoration-graph-planning.md) includes a complete async scope example and explains the native original registration and scoped holder. +The recipe below verifies exactly-once disposal for each scoped layer and preservation of caller ownership. Forwarding `Inner.Dispose()` from `StoreDecorator` would break these assertions. For an async decorator, release only its own async resources in `DisposeAsync()` and use async scope/provider disposal when any layer is async-only. The [detailed design](../../../../docs/decorator-architecture.md) includes a complete async scope example and explains the native original registration and scoped holder. ```csharp using Microsoft.Extensions.DependencyInjection; diff --git a/README.md b/README.md index e98856e..ae0a709 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ Use `Decorate()` to wrap an existing service with a decora > **Decorators must not dispose their injected inner service.** Never forward `Dispose()` or `DisposeAsync()` to the inner service. DI disposes every container-created original and decorator independently; instances supplied through an instance registration remain caller-owned. A decorator releases only resources it creates and owns itself. -See [decorator disposal and ownership](#decorator-disposal-and-ownership) for a safe implementation, and [the detailed design](docs/original-decoration-graph-planning.md) for native activation, scoped holders, factory layers, and the reasons for this registration strategy. +See [decorator disposal and ownership](#decorator-disposal-and-ownership) for a safe implementation, and [the detailed design](docs/decorator-architecture.md) for native activation, scoped holders, factory layers, and the reasons for this registration strategy. Both interface-based and class-based services can be decorated. The following examples demonstrate how to decorate services. @@ -103,7 +103,7 @@ services.Decorate(); Decorators preserve Singleton, Scoped and Transient lifetimes and native keys for type, instance and factory registrations. `Decorate()` wraps only the **last exact service-type registration**, retaining its position. Repeated calls add outer layers; this API does not register open-generic decorators. -Original implementation-type registrations retain native constructor graph planning: invalid nested, generic, enumerable or cyclic graphs fail before dependency factories run. `ValidateOnBuild` can reject them at startup, including inherited-key dependencies validated under `AnyKey`. Factory registrations and non-empty dependency maps remain opaque to native graph inspection. See [the registration design](docs/original-decoration-graph-planning.md). +Original implementation-type registrations retain native constructor graph planning: invalid nested, generic, enumerable or cyclic graphs fail before dependency factories run. `ValidateOnBuild` can reject them at startup, including inherited-key dependencies validated under `AnyKey`. Factory registrations and non-empty dependency maps remain opaque to native graph inspection. See [the registration design](docs/decorator-architecture.md). Each occurrence in the collection is a separate registration, even when the same `ServiceDescriptor` instance is added more than once. Only the last occurrence is decorated: @@ -152,7 +152,7 @@ public sealed class BufferedServiceDecorator(IBufferedService inner) } ``` -Dispose the owning scope/provider to release container-owned layers. Use async scope/provider disposal for async-only resources. For `AddSingleton(existingInstance)` or its keyed instance overload, the caller remains responsible for `existingInstance`; a factory returning an object gives DI ownership of its result. See the [complete keyed ownership example](.agents/skills/mammoth-di/references/usage.md#keyed-decorators-and-caller-owned-instances) and the [design's runnable scoped example](docs/original-decoration-graph-planning.md#the-ownership-rule). +Dispose the owning scope/provider to release container-owned layers. Use async scope/provider disposal for async-only resources. For `AddSingleton(existingInstance)` or its keyed instance overload, the caller remains responsible for `existingInstance`; a factory returning an object gives DI ownership of its result. See the [complete keyed ownership example](.agents/skills/mammoth-di/references/usage.md#keyed-decorators-and-caller-owned-instances) and the [design's runnable scoped example](docs/decorator-architecture.md#the-ownership-rule). ### DependsOn (requires Keyed Services support) diff --git a/docs/original-decoration-graph-planning.md b/docs/decorator-architecture.md similarity index 98% rename from docs/original-decoration-graph-planning.md rename to docs/decorator-architecture.md index 1bf96bc..40d59c6 100644 --- a/docs/original-decoration-graph-planning.md +++ b/docs/decorator-architecture.md @@ -1,4 +1,4 @@ -# Decoration: native activation, caching, and disposal ownership +# Decorator architecture and design decisions This document explains [PR #124](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/pull/124), which fixes [issue #121](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/121). The design restores native Microsoft DI constructor planning for decorated original **implementation-type registrations**, while retaining independent disposal ownership and registration isolation. These changes belong to the release after 0.8.0; they are not guarantees of the published 0.8.0 package. diff --git a/docs/keyed-built-in-registration-probing.md b/docs/keyed-built-in-registration-probing.md index b37efa1..d447a49 100644 --- a/docs/keyed-built-in-registration-probing.md +++ b/docs/keyed-built-in-registration-probing.md @@ -65,7 +65,7 @@ This explanation concerns the new private-DI registration fallback. Constructor/ ## Native original types and diagnostic activation -[Issue #121](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/121) keeps decorated original implementation types in native DI registrations. Native DI now handles their constructor selection, generic constraints and complete inspectable dependency graphs; this path no longer calls Mammoth's original-type activator or its copied-descriptor fallback. Private identities retain the original public key and isolate each registration. See [the registration design](original-decoration-graph-planning.md), including the scoped compiled-cache safeguard. +[Issue #121](https://github.com/PrimordialCode/Mammoth.Extensions.DependencyInjection/issues/121) keeps decorated original implementation types in native DI registrations. Native DI now handles their constructor selection, generic constraints and complete inspectable dependency graphs; this path no longer calls Mammoth's original-type activator or its copied-descriptor fallback. Private identities retain the original public key and isolate each registration. See [the registration design](decorator-architecture.md), including the scoped compiled-cache safeguard. Diagnostic instrumentation still uses `NativeConstructorActivator` for ordinary implementation types after native startup validation. It ignores preferred-constructor attributes and checks parameter-type ambiguity and generic constraints without activating rejected dependencies. Mammoth snapshots are preferred; recognized native ordinary probes reuse the guarded descriptor fallback and provider-specific weak cache. It requests a keyed availability probe only for non-null dependency lookup keys (#120). This fallback adds no private member lookup beyond the descriptor field described above.