Write unit tests for DependencyPropertyChangeNotifier

Stylet/DependencyPropertyChangeNotifier.cs

@@ -9,7 +9,7 @@ namespace Stylet
     /// DependencyProperty change notifier which does not root the DependencyObject
     /// </summary>
     // Adapted from https://agsmith.wordpress.com/2008/04/07/propertydescriptor-addvaluechanged-alternative/
-    public class PropertyChangeNotifier : DependencyObject, IDisposable
+    public class DependencyPropertyChangeNotifier : DependencyObject, IDisposable
     {
         /// <summary>
         /// Watch for changes of the given property on the given propertySource
@@ -18,9 +18,9 @@ namespace Stylet
         /// <param name="property">Property on the object to observe</param>
         /// <param name="handler">Handler to invoke when the property changes</param>
         /// <returns>The constructed PropertyChangeNotifier</returns>
-        public static PropertyChangeNotifier AddValueChanged(DependencyObject propertySource, PropertyPath property, PropertyChangedCallback handler)
+        public static DependencyPropertyChangeNotifier AddValueChanged(DependencyObject propertySource, PropertyPath property, PropertyChangedCallback handler)
         {
-            return new PropertyChangeNotifier(propertySource, property, handler);
+            return new DependencyPropertyChangeNotifier(propertySource, property, handler);
         }
 
         /// <summary>
@@ -30,15 +30,17 @@ namespace Stylet
         /// <param name="property">Property on the object to observe</param>
         /// <param name="handler">Handler to invoke when the property changes</param>
         /// <returns>The constructed PropertyChangeNotifier</returns>
-        public static PropertyChangeNotifier AddValueChanged(DependencyObject propertySource, DependencyProperty property, PropertyChangedCallback handler)
+        public static DependencyPropertyChangeNotifier AddValueChanged(DependencyObject propertySource, DependencyProperty property, PropertyChangedCallback handler)
         {
+            if (property == null)
+                throw new ArgumentNullException("property");
             return AddValueChanged(propertySource, new PropertyPath(property), handler);
         }
 
-        private readonly PropertyChangedCallback handler;
+        private PropertyChangedCallback handler;
         private readonly WeakReference<DependencyObject> propertySource;
 
-        private PropertyChangeNotifier(DependencyObject propertySource, PropertyPath property, PropertyChangedCallback handler)
+        private DependencyPropertyChangeNotifier(DependencyObject propertySource, PropertyPath property, PropertyChangedCallback handler)
         {
             if (propertySource == null)
                 throw new ArgumentNullException("propertySource");
@@ -48,7 +50,6 @@ namespace Stylet
                 throw new ArgumentNullException("handler");
 
             this.propertySource = new WeakReference<DependencyObject>(propertySource);
-            this.handler = handler;
 
             var binding = new Binding()
             {
@@ -57,21 +58,29 @@ namespace Stylet
                 Source = propertySource
             };
             BindingOperations.SetBinding(this, ValueProperty, binding);
+
+            // Needs to be set after binding set, so it doesn't catch the initial property set
+            this.handler = handler;
         }
 
         private void OnValueChanged(DependencyPropertyChangedEventArgs e)
         {
+            // This happens on the firsrt invocation ever, when the initial value is set
+            // and on disposal
+            if (this.handler == null)
+                return;
+
             // Target *should* never be null at this point...
-            DependencyObject propertySource;
-            if (!this.propertySource.TryGetTarget(out propertySource))
-                Debug.Assert(false);
+            DependencyObject propertySource = null;
+            this.propertySource.TryGetTarget(out propertySource);
+            Debug.Assert(propertySource != null);
             this.handler(propertySource, e);
         }
 
         private static readonly DependencyProperty ValueProperty =
-            DependencyProperty.Register("Value", typeof(object), typeof(PropertyChangeNotifier), new FrameworkPropertyMetadata(null, (d, e) =>
+            DependencyProperty.Register("Value", typeof(object), typeof(DependencyPropertyChangeNotifier), new FrameworkPropertyMetadata(null, (d, e) =>
             {
-                ((PropertyChangeNotifier)d).OnValueChanged(e);
+                ((DependencyPropertyChangeNotifier)d).OnValueChanged(e);
             }));
 
         /// <summary>
@@ -79,6 +88,7 @@ namespace Stylet
         /// </summary>
         public void Dispose()
         {
+            this.handler = null; // Otherwise it's called as the binding is unset
             BindingOperations.ClearBinding(this, ValueProperty);
         }
     }

Stylet/Stylet.csproj

@@ -52,7 +52,7 @@
     <Compile Include="Logging\ILogger.cs" />
     <Compile Include="Logging\NullLogger.cs" />
     <Compile Include="Logging\TraceLogger.cs" />
-    <Compile Include="PropertyChangeNotifier.cs" />
+    <Compile Include="DependencyPropertyChangeNotifier.cs" />
     <Compile Include="StyletIoC\Creation\ICreator.cs" />
     <Compile Include="StyletIoC\Creation\IRegistration.cs" />
     <Compile Include="StyletIoC\Internal\Builders\BuilderAbstractFactoryBinding.cs" />

Stylet/Xaml/CommandAction.cs

@@ -63,7 +63,7 @@ namespace Stylet.Xaml
             this.UpdateGuardAndMethod();
 
             // Observe the View.ActionTarget for changes, and re-bind the guard property and MethodInfo if it changes
-            PropertyChangeNotifier.AddValueChanged(this.Subject, View.ActionTargetProperty, (o, e) => this.UpdateGuardAndMethod());
+            DependencyPropertyChangeNotifier.AddValueChanged(this.Subject, View.ActionTargetProperty, (o, e) => this.UpdateGuardAndMethod());
         }
 
         private string GuardName

Stylet/Xaml/EventAction.cs

@@ -64,7 +64,7 @@ namespace Stylet.Xaml
             this.UpdateMethod();
 
             // Observe the View.ActionTarget for changes, and re-bind the guard property and MethodInfo if it changes
-            PropertyChangeNotifier.AddValueChanged(this.subject, View.ActionTargetProperty, (o, e) => this.UpdateMethod());
+            DependencyPropertyChangeNotifier.AddValueChanged(this.subject, View.ActionTargetProperty, (o, e) => this.UpdateMethod());
         }
 
         private void UpdateMethod()

StyletUnitTests/DependencyPropertyChangeNotifierTests.cs

@@ -0,0 +1,100 @@
+using NUnit.Framework;
+using Stylet;
+using Stylet.Xaml;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using System.Windows;
+
+namespace StyletUnitTests
+{
+    [TestFixture]
+    public class DependencyPropertyChangeNotifierTests
+    {
+        [Test]
+        public void ThrowsIfTargetIsNull()
+        {
+            Assert.Throws<ArgumentNullException>(() => DependencyPropertyChangeNotifier.AddValueChanged(null, View.ActionTargetProperty, (d, e) => { }));
+        }
+
+        [Test]
+        public void ThrowsIfPropertyIsNull()
+        {
+            Assert.Throws<ArgumentNullException>(() => DependencyPropertyChangeNotifier.AddValueChanged(new DependencyObject(), (PropertyPath)null, (d, e) => { }));
+            Assert.Throws<ArgumentNullException>(() => DependencyPropertyChangeNotifier.AddValueChanged(new DependencyObject(), (DependencyProperty)null, (d, e) => { }));
+        }
+
+        [Test]
+        public void ThrowsIfHandlerIsNull()
+        {
+            Assert.Throws<ArgumentNullException>(() => DependencyPropertyChangeNotifier.AddValueChanged(new DependencyObject(), View.ActionTargetProperty, null));
+        }
+
+        [Test]
+        public void DoesNotRetainTarget()
+        {
+            var target = new DependencyObject();
+            var weakTarget = new WeakReference(target);
+
+            DependencyPropertyChangeNotifier.AddValueChanged(target, View.ActionTargetProperty, (d, e) => { });
+
+            target = null;
+            GC.Collect();
+
+            Assert.IsFalse(weakTarget.IsAlive);
+        }
+
+        [Test]
+        public void NotifiesOfChange()
+        {
+            var view = new DependencyObject();
+
+            var value1 = new object();
+            var value2 = new object();
+
+            View.SetActionTarget(view, value1);
+
+            DependencyObject subject = null;
+            DependencyPropertyChangedEventArgs ea = default(DependencyPropertyChangedEventArgs);
+            DependencyPropertyChangeNotifier.AddValueChanged(view, View.ActionTargetProperty, (d, e) =>
+            {
+                subject = d;
+                ea = e;
+            });
+
+            View.SetActionTarget(view, value2);
+
+            Assert.AreEqual(view, subject);
+            Assert.AreEqual(value1, ea.OldValue);
+            Assert.AreEqual(value2, ea.NewValue);
+        }
+
+        [Test]
+        public void HandlerNotCalledBeforeDependencyPropertyChanged()
+        {
+            var view = new DependencyObject();
+
+            var called = false;
+            DependencyPropertyChangeNotifier.AddValueChanged(view, View.ActionTargetProperty, (d, e) => called = true);
+
+            Assert.IsFalse(called);
+        }
+
+        [Test]
+        public void DisposeUnsubscribes()
+        {
+            var view = new DependencyObject();
+
+            var called = false;
+            var disposable = DependencyPropertyChangeNotifier.AddValueChanged(view, View.ActionTargetProperty, (d, e) => called = true);
+
+            disposable.Dispose();
+
+            View.SetActionTarget(view, new object());
+
+            Assert.IsFalse(called);
+        }
+    }
+}

StyletUnitTests/StyletUnitTests.csproj

@@ -64,6 +64,7 @@
     <Compile Include="ConductorOneActiveTests.cs" />
     <Compile Include="ConductorTests.cs" />
     <Compile Include="DebugConverterTests.cs" />
+    <Compile Include="DependencyPropertyChangeNotifierTests.cs" />
     <Compile Include="StyletIoC\StyletIoCFuncFactoryTests.cs" />
     <Compile Include="StyletIoC\StyletIoCInstanceBindingTests.cs" />
     <Compile Include="StyletIoC\StyletIoCModuleTests.cs" />