Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix and maybe optimize CopyProperty for inherited properties #1850

Merged
merged 1 commit into from
Sep 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 20 additions & 9 deletions src/Framework/Framework/Controls/DotvvmBindableObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -403,24 +403,35 @@ public virtual IEnumerable<DotvvmBindableObject> GetLogicalChildren()
/// <param name="target">The <see cref="DotvvmBindableObject"/> that holds the value of the <paramref name="targetProperty"/></param>
/// <param name="targetProperty">The <see cref="DotvvmProperty"/> to which <paramref name="sourceProperty"/> will be copied</param>
/// <param name="throwOnFailure">Determines whether to throw an exception if copying fails</param>
protected void CopyProperty(DotvvmProperty sourceProperty, DotvvmBindableObject target, DotvvmProperty targetProperty, bool throwOnFailure = false)
protected internal void CopyProperty(DotvvmProperty sourceProperty, DotvvmBindableObject target, DotvvmProperty targetProperty, bool throwOnFailure = false)
{
if (throwOnFailure && !targetProperty.MarkupOptions.AllowBinding && !targetProperty.MarkupOptions.AllowHardCodedValue)
var targetOptions = targetProperty.MarkupOptions;
if (throwOnFailure && !targetOptions.AllowBinding && !targetOptions.AllowHardCodedValue)
{
throw new DotvvmControlException(this, $"TargetProperty: {targetProperty.FullName} doesn't allow bindings nor hard coded values");
}

if (targetProperty.MarkupOptions.AllowBinding && HasBinding(sourceProperty))
if (IsPropertySet(sourceProperty))
{
target.SetBinding(targetProperty, GetBinding(sourceProperty));
}
else if (targetProperty.MarkupOptions.AllowHardCodedValue && IsPropertySet(sourceProperty))
{
target.SetValue(targetProperty, GetValue(sourceProperty));
var sourceValue = GetValueRaw(sourceProperty);
if ((targetOptions.AllowBinding || sourceValue is not IBinding) &&
(targetOptions.AllowHardCodedValue || sourceValue is IBinding))
{
target.SetValueRaw(targetProperty, sourceValue);
}
else if (targetOptions.AllowHardCodedValue)
{
target.SetValue(targetProperty, EvalPropertyValue(sourceProperty, sourceValue));
}
else if (throwOnFailure)
{
throw new DotvvmControlException(this, $"Value of {sourceProperty.FullName} couldn't be copied to targetProperty: {targetProperty.FullName}, because {targetProperty.FullName} does not support hard coded values.");
}
}

else if (throwOnFailure)
{
throw new DotvvmControlException(this, $"Value of {sourceProperty.FullName} couldn't be copied to targetProperty: {targetProperty.FullName}, because {targetProperty.FullName} is not set.");
throw new DotvvmControlException(this, $"Value of {sourceProperty.FullName} couldn't be copied to targetProperty: {targetProperty.FullName}, because {sourceProperty.FullName} is not set.");
}
}
}
Expand Down
174 changes: 174 additions & 0 deletions src/Tests/Runtime/DotvvmBindableObjectTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
using System.Threading.Tasks;
using DotVVM.Framework.Binding;
using DotVVM.Framework.Compilation.ControlTree;
using DotVVM.Framework.Controls;
using DotVVM.Framework.Testing;
using DotVVM.Framework.Tests.Binding;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace DotVVM.Framework.Tests.Runtime
{
[TestClass]
public class DotvvmBindableObjectTests
{
readonly BindingCompilationService bindingService = DotvvmTestHelper.DefaultConfig.ServiceProvider.GetRequiredService<BindingCompilationService>();
readonly DataContextStack dataContext = DataContextStack.Create(typeof(TestViewModel));

[TestMethod]
public void CopyProperty_Error_NotSet()
{
var source = new HtmlGenericControl("div");
var target = new HtmlGenericControl("div");

var ex = Assert.ThrowsException<DotvvmControlException>(() => source.CopyProperty(HtmlGenericControl.VisibleProperty, target, HtmlGenericControl.VisibleProperty, throwOnFailure: true));
StringAssert.Contains(ex.Message, "Visible is not set");
}

[TestMethod]
public void CopyProperty_Nop_NotSet()
{
var source = new HtmlGenericControl("div");
var target = new HtmlGenericControl("div");

source.CopyProperty(HtmlGenericControl.VisibleProperty, target, HtmlGenericControl.VisibleProperty); // throwOnFailure: false is default
Assert.IsFalse(target.IsPropertySet(HtmlGenericControl.VisibleProperty));
}

[TestMethod]
public void CopyProperty_Copy_Value()
{
var source = new HtmlGenericControl("div");
source.SetValue(HtmlGenericControl.VisibleProperty, (object)false);
var target = new HtmlGenericControl("div");
source.CopyProperty(HtmlGenericControl.VisibleProperty, target, HtmlGenericControl.VisibleProperty);

Assert.IsFalse(target.GetValue<bool>(HtmlGenericControl.VisibleProperty));
Assert.AreSame(source.GetValue(HtmlGenericControl.VisibleProperty), target.GetValue(HtmlGenericControl.VisibleProperty));
}

[TestMethod]
public void CopyProperty_Copy_Binding()
{
var source = new HtmlGenericControl("div");
source.DataContext = new TestViewModel { IntProp = 0 };
source.SetValue(Internal.DataContextTypeProperty, dataContext);
source.SetValue(HtmlGenericControl.VisibleProperty, bindingService.Cache.CreateValueBinding("IntProp == 12", dataContext));
var target = new HtmlGenericControl("div");
source.CopyProperty(HtmlGenericControl.VisibleProperty, target, HtmlGenericControl.VisibleProperty);
target.DataContext = source.DataContext;

Assert.IsFalse(source.GetValue<bool>(HtmlGenericControl.VisibleProperty));
Assert.IsFalse(target.GetValue<bool>(HtmlGenericControl.VisibleProperty));
Assert.AreSame(source.GetValue(HtmlGenericControl.VisibleProperty), target.GetValue(HtmlGenericControl.VisibleProperty));
}

[TestMethod]
public void CopyProperty_EvalBinding()
{
var source = new HtmlGenericControl("div");
source.DataContext = new TestViewModel { IntProp = 0 };
source.SetValue(Internal.DataContextTypeProperty, dataContext);
source.SetValue(HtmlGenericControl.VisibleProperty, bindingService.Cache.CreateValueBinding("IntProp == 12", dataContext));

Assert.IsFalse(Button.IsSubmitButtonProperty.MarkupOptions.AllowBinding);
var target = new Button();
source.CopyProperty(HtmlGenericControl.VisibleProperty, target, Button.IsSubmitButtonProperty);
target.DataContext = source.DataContext;

Assert.IsFalse(target.IsSubmitButton);
Assert.AreEqual(false, target.GetValueRaw(Button.IsSubmitButtonProperty));
}

[TestMethod]
public void CopyProperty_Error_ValueToBinding()
{
var source = new HtmlGenericControl("div");
source.SetValue(HtmlGenericControl.VisibleProperty, (object)false);
Assert.IsFalse(CheckBox.CheckedProperty.MarkupOptions.AllowHardCodedValue);
var target = new CheckBox();

var ex = Assert.ThrowsException<DotvvmControlException>(() =>
source.CopyProperty(HtmlGenericControl.VisibleProperty, target, CheckBox.CheckedProperty, throwOnFailure: true));
StringAssert.Contains(ex.Message, "Checked does not support hard coded values");
}

[TestMethod]
public void CopyProperty_Nop_ValueToBinding()
{
// TODO: this is a weird behavior, I'd consider changing it in a future major version
var source = new HtmlGenericControl("div");
source.SetValue(HtmlGenericControl.VisibleProperty, (object)false);
Assert.IsFalse(CheckBox.CheckedProperty.MarkupOptions.AllowHardCodedValue);
var target = new CheckBox();

source.CopyProperty(HtmlGenericControl.VisibleProperty, target, CheckBox.CheckedProperty);
Assert.IsFalse(target.IsPropertySet(CheckBox.CheckedProperty));
}

[TestMethod]
public void CopyProperty_Copy_InheritedBinding()
{
var sourceParent = new HtmlGenericControl("div");
sourceParent.DataContext = new TestViewModel { IntProp = 0 };
sourceParent.SetValue(Internal.DataContextTypeProperty, dataContext);
sourceParent.SetValue(Validation.EnabledProperty, bindingService.Cache.CreateValueBinding("IntProp == 12", dataContext));
var source = new HtmlGenericControl("div");
sourceParent.Children.Add(source);

var target = new HtmlGenericControl("div");
source.CopyProperty(Validation.EnabledProperty, target, Validation.EnabledProperty);

Assert.AreSame(sourceParent.GetValue(Validation.EnabledProperty), target.GetValue(Validation.EnabledProperty));
}

[TestMethod]
public void CopyProperty_Copy_InheritedValue()
{
var sourceParent = new HtmlGenericControl("div");
sourceParent.SetValue(Validation.EnabledProperty, (object)false);
var source = new HtmlGenericControl("div");
sourceParent.Children.Add(source);

var target = new HtmlGenericControl("div");
source.CopyProperty(Validation.EnabledProperty, target, Validation.EnabledProperty);

Assert.AreSame(sourceParent.GetValue(Validation.EnabledProperty), target.GetValue(Validation.EnabledProperty));
}


[TestMethod]
public void CopyProperty_Copy_FormControlsEnabledBinding()
{
var sourceParent = new HtmlGenericControl("div");
sourceParent.DataContext = new TestViewModel { IntProp = 0 };
sourceParent.SetValue(Internal.DataContextTypeProperty, dataContext);
sourceParent.SetValue(FormControls.EnabledProperty, bindingService.Cache.CreateValueBinding("IntProp == 12", dataContext));
var source = new TextBox();
sourceParent.Children.Add(source);

var target = new TextBox();
source.CopyProperty(TextBox.EnabledProperty, target, TextBox.EnabledProperty);
target.DataContext = source.DataContext;

Assert.AreSame(sourceParent.GetValue(FormControls.EnabledProperty), target.GetValue(TextBox.EnabledProperty));
Assert.AreSame(source.GetValue(TextBox.EnabledProperty), target.GetValue(TextBox.EnabledProperty));
}

[TestMethod]
public void CopyProperty_Copy_FormControlsEnabledValue()
{
var sourceParent = new HtmlGenericControl("div");
sourceParent.SetValue(FormControls.EnabledProperty, (object)false);
var source = new TextBox();
sourceParent.Children.Add(source);

var target = new TextBox();
source.CopyProperty(TextBox.EnabledProperty, target, TextBox.EnabledProperty);

Assert.AreSame(sourceParent.GetValue(FormControls.EnabledProperty), target.GetValue(TextBox.EnabledProperty));
Assert.AreSame(source.GetValue(TextBox.EnabledProperty), target.GetValue(TextBox.EnabledProperty));
}

}
}
Loading