dotvvm
dotvvm copied to clipboard
Static command don't resolve viewmodel correctly
public class SomeDependency
{
public string SomeProperty { get; set; }
}
public class MasterViewModel : DotvvmViewModelBase
{
private string dependencySomeProperty;
public MasterViewModel(SomeDependency dependency)
{
//System.NullReferenceExceptionObject reference not set to an instance of an object.
dependencySomeProperty = dependency.SomeProperty;
}
}
public class PageViewModel : MasterViewModel
{
public PageViewModel(SomeDependency dependency) : base(dependency)
{
}
[AllowStaticCommand]
public void StaticCommand()
{
}
}
Dependencies are null
when static command is executed so I get NullReferenceException in constructor.
DotVVM version: 2.4.0.1
Calling instance methods on viewmodels from static commands is not supported. You should use Static Command Services.
Since the static commands don't send the viewmodel to the server, property values of the viewmodel are unknown and the object will be in a strange and probably inconsistent state. The same applies to the dependencies passed in the constructor - for historical reasons, they are set to null because it was implemented before DotVVM was using IServiceProvider
.
We could "fix" this by resolving them from the IServiceProvider
, but still - the viewmodel will be in a weird state, the lifecycle methods won't be called, and the method may rely on data which are not there.
We should throw an exception in this case - I think that the probability of using this feature by mistake is greater than the probability that someone is using it intentionally and is happy with how it behaves.
Oh, I see. I think the exception is best solution of this
dependencies passed in the constructor - for historical reasons, they are set to null because it was implemented before DotVVM was using IServiceProvider.
I don't think so, the arguments of a static command are simply deserialized using Newtonsoft.Json and, apparently it passes nulls to constructors (if a property of the same name does not exist).
This is how arguments are processed: https://github.com/riganti/dotvvm/blob/master/src/DotVVM.Framework/Hosting/DotvvmPresenter.cs#L332
@tomasherceg What can we do about this? Shall we also use the dotvvm serializer static command arguments? (this is certainly a breaking change) Or call it a "feature"?
I think that the only reason for using an instance method as a target of static commands is the dependency injection, so we can change the behavior and make a breaking change here - I don't think we'll break someone as this entire situation is quite rare, and if the viewmodel instance can be created for the original page load, it should resolve for a static command as well.
Now, I don't know if you mean banning everything except @service
in the instance parameter or introducing a different way to deserialize it by not deserializing it at all and only filling in services.
In any case, it does not really solve the problem, since we are still deserializing the other parameters in this way.
We've discussed this and it is a breaking change so it needs to go to v3.
- Restrict instance method calls only to static command services.
- Parameters should be deserialized with our deserializer.
- We need to restrict signed and encrypted values in parameters - there is no way how to validate them, so we need to throw an exception.