- changed status to open
dynamically created factory function won't correctly handle empty TBytes values and raises EActivatorException
Example:
Container.RegisterType<IInstance, TInstance>;
Container.RegisterFactory<TCreate<IInstance>>;
Container.Build;
Instance := Container.Resolve<IInstance>([TValue.From<TBytes>([])]);
// Empty TBytes, passed through by `Resolve`, works.
CreateInstance := Container.Resolve<TCreate<IInstance>>();
Instance := CreateInstance([1]);
// Non-empty TBytes, passed through by a factory function, works.
Instance := CreateInstance([]);
// Empty TBytes, passed through by a factory function, does not work:
// EActivatorException: Unsatisfied constructor on type: Project1.TInstance
Actual behaviour:
Container.Resolve<IInstance>([TValue.From<TBytes>([])])
returns an instance.CreateInstance([1])
returns an instance.CreateInstance([])
raisesEActivatorException
: “Unsatisfied constructor on type”.
Expected bahaviour:
CreateInstance([])
returns an instance, too.
Additional thoughts:
(1) Given that Container.Resolve<IInstance>([TValue.From<TBytes>([])])
returns an instance as expected, it appears to be the dynamically created factory function that is returned by Container.Resolve<TCreate<IInstance>>
to not correctly handle the (empty) TBytes value.
(2) Stepping through and inspecting TDependencyResolver.CanResolve
(line 253 in Spring.Container.Resolvers.pas) the following reordering seems to make sense, and gives the desired expected behaviour, and does not break any test in the test suite:
@@ -265,14 +265,15 @@ begin
Exit(True);
if CanResolveFromSubResolvers(context, dependency, argument) then
Exit(True);
+ if CanResolveFromArgument(context, dependency, argument) then
+ Result := True
+ else
if argument.IsEmpty then
Result := Kernel.Registry.HasDefault(dependency.TypeInfo)
- else if CanResolveFromArgument(context, dependency, argument) then
- Result := True
else if argument.TryAsType(TypeInfo(TTypeKind), kind) and (kind = tkDynArray) then
Result := Kernel.Registry.HasService(dependency.TypeInfo)
else
begin
Result := argument.IsString;
Conclusion
Knowing about (1) makes (2) appear to be the wrong change to fix this issue, and I’m left to think that the dynamically created factory function won't correctly handle empty TBytes values.
System information:
-
Spring4D commit 409cf53, which is the most recent commit in the branch "develop". – Also checked and affected:
- Spring4D commit 6211f45, which ist the most recent commit in the branch “feature/container-refactoring”. Then the reordering happens at line 243 in Spring.Container.Resolvers.pas with the same effects.
-
Delphi 10.3
Attachments:
Demo.dpr
contains a complete demonstration.
Comments (7)
-
repo owner -
repo owner I looked into the Demo again and you can see that in (1) it does not call the correct ctor but falls back onto the parameterless one (which it should not see but thats another issue with discovering ctors via RTTI) - making this also behave wrong and mistakenly not to consider to pass an empty dynamic array to the proper ctor.
-
repo owner Ugh - I see - fixing https://quality.embarcadero.com/browse/RSP-13747 opened another can of worms: the fact that TValue.IsType returns true for any type if the value says IsEmpty which it should not.
-
repo owner - changed status to resolved
fixed
#336(fixed casting an "empty" TValue to any other type)→ <<cset 80e690264d93>>
-
reporter Indeed, I did not catch the fallback to the parameterless constructor when instantiating through
Resolve
directly. I came up with that line only in the Demo for comparison and instead use the factory function as the way to go in my code, from where this issue originated. Thank you very much for the responsive fixing! Also in my code, things work as expected now. And thank you very much for the mainenance and development of this very useful library, too! -
repo owner fixed
#336(fixed casting an "empty" TValue to any other type)→ <<cset c50000c09a23>>
-
repo owner fixed
#336(fixed casting an "empty" TValue to any other type)→ <<cset 580a5ec26256>>
- Log in to comment
Thanks for the detailed report and the analysis - I am sure you are spot on with the fix. I will verify and fix it as soon as possible in develop