Skip to content

Fix registered null constructor dependencies (#110) - #117

Merged
AGiorgetti merged 3 commits into
developfrom
codex/fix-110-null-factories
Oct 8, 2026
Merged

AGiorgetti merged 3 commits into
developfrom
codex/fix-110-null-factories

Conversation

@AGiorgetti

@AGiorgetti AGiorgetti commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Adding an unrelated DependsOn map currently makes a registered factory's null result throw a misleading missing-service exception. Preserve that result for ordinary, attribute-keyed and named-key dependencies, including contextual keyed decorators.

The fix changes three required-service lookups to GetService/GetKeyedService after constructor selection has already checked registration availability. Missing optional registrations still use their defaults; missing required registrations still fail before dependency activation; application factory exceptions retain their original identity. Adds an explanatory source comment and a vNext changelog entry.

Closes #110.

Verification

Final commit: f5e4c5351416db4fe446c568d50088830ebf48d9.

  • Regression tests added before production changes: 84 failed and 30 passed on each of net472, net8.0, net9.0 and net10.0. The corrected final fixture was also rechecked against pre-fix source with the same results.
  • Final Release restore and solution build passed, including netstandard2.0.
  • Full suite: 1,978 passed per target, 7,912 total; zero failed or skipped.
  • 114 new cases per target cover required nullable references, non-null optional reference and nullable-value defaults, ordinary/explicit/null/inherited/named keys, all dependency lifetimes, native/snapshot/diagnostic providers, repeated resolutions in two scopes, contextual decorators, missing registrations and factory exception identity.
  • Native DI hot-cache probes passed on all four targets: 100 interleaved enumerations after observed native accessor compilation.
  • Windows net472 executed on .NET Framework 4.8.9345.0; other local runtimes were .NET 8.0.31, 9.0.20 and 10.0.12.
  • Whitespace check passed. Build emitted only the existing net472 support warnings from Microsoft.Extensions.Telemetry.Abstractions 10.0.0 and Microsoft.Extensions.Diagnostics.Testing 10.0.0; no compiler/analyzer warnings.
  • GitHub CI run 37794439913 passed for this exact head. Logs confirm 1,978 passing tests on each of all four targets and all four native cache probes. Pack and Publish were skipped. No applicable check was unavailable.

Native singleton-null consideration

DI 10's native constructor call-site path may retry a singleton factory that returns null, while its public GetService accessor caches that null. Mammoth continues to use public service lookups and preserves their caching behavior. Tests compare injected values for every lifetime, assert one public singleton factory call, and compare exact scoped/transient call counts with independent native controls. This PR does not change the provider's caching implementation. Relevant upstream paths: runtime constructor/root cache resolution and public singleton accessor creation.

Prepared for review; work will stop after verification until explicitly authorized to continue.

Co-authored-by: Codex codex@openai.com

AGiorgetti and others added 3 commits October 8, 2026 16:34
Co-authored-by: Codex <codex@openai.com>
Co-authored-by: Codex <codex@openai.com>

Copy link
Copy Markdown
Contributor Author

Implemented #110 in PR #117, awaiting review at commit f5e4c5351416db4fe446c568d50088830ebf48d9.

Constructor selection already checks whether ordinary, attribute-keyed and named-key dependencies are registered. Required-service lookups then incorrectly treated a registered factory's null result as a missing registration. The fix uses GetService/GetKeyedService after those availability checks, preserving null for constructor injection. Optional defaults still apply only when registrations are missing; missing required dependencies still fail before activating factories; application exceptions propagate with their original identity. The shared path also covers contextual keyed decorators. An explanatory source comment and vNext changelog entry are included.

Regression tests were added first and verified against pre-fix code on net472/net8.0/net9.0/net10.0: 84 failed and 30 controls passed per target. The 114 new cases cover nullable references and nullable values with non-null defaults, all key modes, all dependency lifetimes, native/snapshot/diagnostic providers, repeated resolutions across scopes, decoration, missing dependencies and factory exceptions.

Final local Release restore/build/full tests passed: 1,978 tests per target, 7,912 total, zero failures/skips. All four native hot-cache probes passed 100 interleaved enumerations after native compilation. Windows net472 ran on .NET Framework 4.8.9345.0. The build has only the two existing net472 support warnings for Telemetry.Abstractions and Diagnostics.Testing 10.0.0; no compiler/analyzer warnings. No applicable check was unavailable.

CI run 37794439913 passed for the exact final commit; downloaded logs confirm those test totals and all four native probes. Pack and Publish were skipped.

The PR documents an upstream caching nuance: native constructor call sites can retry null-returning singleton factories, while public service accessors cache null. Mammoth retains public lookup caching; scoped/transient factory-call counts match native controls.

Stopping here for your review and explicit instruction to continue.

@AGiorgetti
AGiorgetti marked this pull request as ready for review October 8, 2026 14:55
@AGiorgetti
AGiorgetti merged commit 1706e7f into develop Oct 8, 2026
2 checks passed
@AGiorgetti
AGiorgetti deleted the codex/fix-110-null-factories branch October 8, 2026 15:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DependsOn rejects registered null factory results for nullable constructor dependencies

1 participant