Preserve optional constructor argument defaults on class proxies - #68
Merged
Conversation
Class interception rewrites the registered implementation type to a Castle-generated proxy subclass. The generated constructors mirror the parameters of the proxied type but do not carry their default values, so Autofac's DefaultValueParameter cannot see them and optional arguments fail to bind with "None of the constructors found ... can be invoked with the available services and parameters." Supply the missing defaults from the type that was proxied. This is a last resort: resolve-time parameters, parameters configured on the registration, and services available from the container all still take precedence, so binding behavior matches an unproxied registration. Reported downstream as autofac/Autofac.ServiceFabric#41, where every service registration is class-intercepted and any optional constructor argument therefore fails.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #68 +/- ##
===========================================
+ Coverage 90.71% 93.29% +2.58%
===========================================
Files 2 3 +1
Lines 140 194 +54
Branches 23 37 +14
===========================================
+ Hits 127 181 +54
Misses 6 6
Partials 7 7 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Matching a proxy constructor parameter to the proxied type by name and type alone searched every constructor, so overloads sharing a parameter name and type but declaring different defaults could supply the wrong value. Match the whole mirrored signature instead, using the count of leading arguments the proxy takes for itself. Also guard the default value read against the DateTime FormatException that Autofac's DefaultValueParameter handles.
Walking the proxied type's constructors inside CanSupplyValue meant reflecting over them on every resolve of a class-intercepted component, and allocating a fresh parameter, wrapper array and closure each time. Read the defaults into a lookup keyed by the proxy constructor parameter, once, on the first resolve, so binding is a dictionary hit. For a class-intercepted type with an optional argument this drops a resolve from 835 ns / 2.78 KB to 759 ns / 2.48 KB; the overhead on an intercepted type with no arguments falls from 133 to 72 bytes. Adds a benchmark covering the scenario. Drops the DateTime FormatException guard along the way: it covers a .NET Core 1.x reflection bug that no framework this package targets can hit, and the block can't be exercised by a test. Tests cover the constructor matching that used to live in the resolve path, including overloads that take the same number of arguments.
Optional constructor arguments on class proxies bind to their defaults now, so a constructor that previously couldn't bind can be selected where a narrower overload used to win. That changes behavior for applications resolving successfully today, which is more than a patch should carry.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Class interception rewrites
ActivatorData.ImplementationTypeto a Castle-generated proxy subclass. The generated constructors mirror the parameters of the proxied type, but they do not carry those parameters' default values, so Autofac'sDefaultValueParametercan't see them and optional constructor arguments fail to bind:The same registration without
EnableClassInterceptors()resolves fine, which is what makes this surprising: adding interception silently changes constructor binding.This was reported downstream as autofac/Autofac.ServiceFabric#41. That integration class-intercepts every service registration in order to dispose the per-replica lifetime scope, so any Service Fabric service with an optional constructor argument is unresolvable. Nothing about it is Service Fabric specific though — it reproduces with a plain
RegisterType<T>().EnableClassInterceptors().Fix
Keep a reference to the type being proxied and supply its constructor default values on the proxy's behalf via a new internal
ProxiedDefaultValueParameter.It deliberately behaves as a last resort, deferring to everything that would otherwise win so that binding matches an unproxied registration:
WithParameter)The precedence checks matter because parameters added through
OnPreparingare evaluated ahead of the activator's ownAutowiringParameter/DefaultValueParameter. Without them this would shadow real registrations and configured parameters, silently substituting defaults for values the user supplied.Required parameters that genuinely can't be satisfied still throw as before, so real misconfigurations aren't masked.
Notes
NotNullWhenAttributepolyfill fornetstandard2.0, following the existing pattern inAutofac.Extensions.DependencyInjection, since Autofac's own copy isinternal.Testing
New fixture
ClassInterceptorsWithOptionalParametersFixturecovers optional reference and value type arguments, each of the three precedence rules above, that interception still applies when an argument is defaulted, and that a missing required argument still throws.Full build green on all four target frameworks: 53 tests pass (46 existing, 7 new), zero warnings.
Also verified end to end against the reporting scenario by temporarily project-referencing this branch from Autofac.ServiceFabric: a
StatelessServicewith(StatelessServiceContext context, IDependency? optional = null, int count = 42)fails to resolve ondevelopand resolves correctly with this change.