Merge pull request #1850 from riganti/fix-CopyProperty-formcontrolsen…

…abled Fix and maybe optimize CopyProperty for inherited properties
riganti · Sep 7, 2024 · b22dab3 · b22dab3
2 parents d1825a5 + 75cb710
commit b22dab3
Show file tree

Hide file tree

Showing 2 changed files with 194 additions and 9 deletions.
diff --git a/src/Framework/Framework/Controls/DotvvmBindableObject.cs b/src/Framework/Framework/Controls/DotvvmBindableObject.cs
@@ -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.");
             }
         }
     }

diff --git a/src/Tests/Runtime/DotvvmBindableObjectTests.cs b/src/Tests/Runtime/DotvvmBindableObjectTests.cs
@@ -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));
+        }
+
+    }
+}