diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 46143b9..79015f5 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,11 @@ Stylet Changelog ================ +v1.1.3 +------ + + - Fix issue where actions may not fire. Rare case caused by fix from v1.1.2. + v1.1.2 ------ diff --git a/NuGet/Stylet.nuspec b/NuGet/Stylet.nuspec index 84dccb9..d3f85e8 100644 --- a/NuGet/Stylet.nuspec +++ b/NuGet/Stylet.nuspec @@ -2,7 +2,7 @@ Stylet - 1.1.2 + 1.1.3 Stylet Antony Male Antony Male diff --git a/Stylet/DependencyPropertyChangeNotifier.cs b/Stylet/DependencyPropertyChangeNotifier.cs deleted file mode 100644 index 9bf28aa..0000000 --- a/Stylet/DependencyPropertyChangeNotifier.cs +++ /dev/null @@ -1,95 +0,0 @@ -using System; -using System.Diagnostics; -using System.Windows; -using System.Windows.Data; - -namespace Stylet -{ - /// - /// DependencyProperty change notifier which does not root the DependencyObject - /// - // Adapted from https://agsmith.wordpress.com/2008/04/07/propertydescriptor-addvaluechanged-alternative/ - public class DependencyPropertyChangeNotifier : DependencyObject, IDisposable - { - /// - /// Watch for changes of the given property on the given propertySource - /// - /// Object to observe a property on - /// Property on the object to observe - /// Handler to invoke when the property changes - /// The constructed PropertyChangeNotifier - public static DependencyPropertyChangeNotifier AddValueChanged(DependencyObject propertySource, PropertyPath property, PropertyChangedCallback handler) - { - return new DependencyPropertyChangeNotifier(propertySource, property, handler); - } - - /// - /// Watch for changes of the given property on the given propertySource - /// - /// Object to observe a property on - /// Property on the object to observe - /// Handler to invoke when the property changes - /// The constructed PropertyChangeNotifier - 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 PropertyChangedCallback handler; - private readonly WeakReference propertySource; - - private DependencyPropertyChangeNotifier(DependencyObject propertySource, PropertyPath property, PropertyChangedCallback handler) - { - if (propertySource == null) - throw new ArgumentNullException("propertySource"); - if (property == null) - throw new ArgumentNullException("property"); - if (handler == null) - throw new ArgumentNullException("handler"); - - this.propertySource = new WeakReference(propertySource); - - var binding = new Binding() - { - Path = property, - Mode = BindingMode.OneWay, - 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 = 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(DependencyPropertyChangeNotifier), new FrameworkPropertyMetadata(null, (d, e) => - { - ((DependencyPropertyChangeNotifier)d).OnValueChanged(e); - })); - - /// - /// Releases the binding - /// - public void Dispose() - { - this.handler = null; // Otherwise it's called as the binding is unset - BindingOperations.ClearBinding(this, ValueProperty); - } - } -} diff --git a/Stylet/Properties/AssemblyInfo.cs b/Stylet/Properties/AssemblyInfo.cs index 0740658..a35ee4c 100644 --- a/Stylet/Properties/AssemblyInfo.cs +++ b/Stylet/Properties/AssemblyInfo.cs @@ -35,5 +35,5 @@ using System.Windows.Markup; // You can specify all the values or you can default the Build and Revision Numbers // by using the '*' as shown below: // [assembly: AssemblyVersion("1.0.*")] -[assembly: AssemblyVersion("1.1.2.0")] -[assembly: AssemblyFileVersion("1.1.2.0")] +[assembly: AssemblyVersion("1.1.3.0")] +[assembly: AssemblyFileVersion("1.1.3.0")] diff --git a/Stylet/Stylet.csproj b/Stylet/Stylet.csproj index f351669..5e13292 100644 --- a/Stylet/Stylet.csproj +++ b/Stylet/Stylet.csproj @@ -52,7 +52,6 @@ - @@ -82,6 +81,7 @@ + diff --git a/Stylet/StyletIoC/Internal/Registrations/RegistrationBase.cs b/Stylet/StyletIoC/Internal/Registrations/RegistrationBase.cs index be0db0d..3fdca40 100644 --- a/Stylet/StyletIoC/Internal/Registrations/RegistrationBase.cs +++ b/Stylet/StyletIoC/Internal/Registrations/RegistrationBase.cs @@ -38,6 +38,14 @@ namespace StyletIoC.Internal.Registrations return Expression.Lambda>(this.GetInstanceExpression(registrationContext), registrationContext).Compile(); } + protected void ClearGenerator() + { + lock (this.generatorLock) + { + this.generator = null; + } + } + public abstract Expression GetInstanceExpression(ParameterExpression registrationContext); } } diff --git a/Stylet/StyletIoC/Internal/Registrations/SingletonRegistration.cs b/Stylet/StyletIoC/Internal/Registrations/SingletonRegistration.cs index 87bba3d..5b72e72 100644 --- a/Stylet/StyletIoC/Internal/Registrations/SingletonRegistration.cs +++ b/Stylet/StyletIoC/Internal/Registrations/SingletonRegistration.cs @@ -25,6 +25,7 @@ namespace StyletIoC.Internal.Registrations disposable.Dispose(); this.instance = this.instanceExpression = null; + this.ClearGenerator(); }; } diff --git a/Stylet/Xaml/ActionBase.cs b/Stylet/Xaml/ActionBase.cs new file mode 100644 index 0000000..056365e --- /dev/null +++ b/Stylet/Xaml/ActionBase.cs @@ -0,0 +1,164 @@ +using Stylet.Logging; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using System.Text; +using System.Threading.Tasks; +using System.Windows; +using System.Windows.Data; + +namespace Stylet.Xaml +{ + /// + /// Common base class for CommandAction and EventAction + /// + public abstract class ActionBase : DependencyObject + { + private readonly ILogger logger; + + /// + /// Gets the View to grab the View.ActionTarget from + /// + public DependencyObject Subject { get; private set; } + + /// + /// Gets the method name. E.g. if someone's gone Buttom Command="{s:Action MyMethod}", this is MyMethod. + /// + public string MethodName { get; private set; } + + /// + /// Gets the MethodInfo for the method to call. This has to exist, or we throw a wobbly + /// + protected MethodInfo TargetMethodInfo { get; private set; } + + /// + /// Behaviour for if the target is null + /// + protected readonly ActionUnavailableBehaviour TargetNullBehaviour; + + /// + /// Behaviour for if the action doesn't exist on the target + /// + protected readonly ActionUnavailableBehaviour ActionNonExistentBehaviour; + + /// + /// Gets the object on which methods will be invokced + /// + public object Target + { + get { return (object)GetValue(targetProperty); } + } + + private static readonly DependencyProperty targetProperty = + DependencyProperty.Register("target", typeof(object), typeof(ActionBase), new PropertyMetadata(null, (d, e) => + { + ((ActionBase)d).UpdateActionTarget(e.OldValue, e.NewValue); + })); + + /// + /// Initialises a new instance of the class + /// + /// View to grab the View.ActionTarget from + /// Method name. the MyMethod in Buttom Command="{s:Action MyMethod}". + /// Behaviour for it the relevant View.ActionTarget is null + /// Behaviour for if the action doesn't exist on the View.ActionTarget + /// Logger to use + public ActionBase(DependencyObject subject, string methodName, ActionUnavailableBehaviour targetNullBehaviour, ActionUnavailableBehaviour actionNonExistentBehaviour, ILogger logger) + { + this.Subject = subject; + this.MethodName = methodName; + this.TargetNullBehaviour = targetNullBehaviour; + this.ActionNonExistentBehaviour = actionNonExistentBehaviour; + this.logger = logger; + + var binding = new Binding() + { + Path = new PropertyPath(View.ActionTargetProperty), + Mode = BindingMode.OneWay, + Source = this.Subject, + }; + BindingOperations.SetBinding(this, targetProperty, binding); + } + + private void UpdateActionTarget(object oldTarget, object newTarget) + { + MethodInfo targetMethodInfo = null; + + // If it's being set to the initial value, ignore it + // At this point, we're executing the View's InitializeComponent method, and the ActionTarget hasn't yet been assigned + // If they've opted to throw if the target is null, then this will cause that exception. + // We'll just wait until the ActionTarget is assigned, and we're called again + if (newTarget == View.InitialActionTarget) + { + return; + } + + if (newTarget == null) + { + // If it's Enable or Disable we don't do anything - CanExecute will handle this + if (this.TargetNullBehaviour == ActionUnavailableBehaviour.Throw) + { + var e = new ActionTargetNullException(String.Format("ActionTarget on element {0} is null (method name is {1})", this.Subject, this.MethodName)); + this.logger.Error(e); + throw e; + } + else + { + this.logger.Info("ActionTarget on element {0} is null (method name is {1}), but NullTarget is not Throw, so carrying on", this.Subject, this.MethodName); + } + } + else + { + var newTargetType = newTarget.GetType(); + + this.OnNewNonNullTarget(newTarget, newTargetType); + + targetMethodInfo = newTargetType.GetMethod(this.MethodName); + + if (targetMethodInfo == null) + { + if (this.ActionNonExistentBehaviour == ActionUnavailableBehaviour.Throw) + { + var e = new ActionNotFoundException(String.Format("Unable to find method {0} on {1}", this.MethodName, newTargetType.Name)); + this.logger.Error(e); + throw e; + } + else + { + this.logger.Warn("Unable to find method {0} on {1}, but ActionNotFound is not Throw, so carrying on", this.MethodName, newTargetType.Name); + } + } + else + { + this.AssertTargetMethodInfo(targetMethodInfo, newTargetType); + } + } + + this.TargetMethodInfo = targetMethodInfo; + + this.OnTargetChanged(oldTarget, newTarget); + } + + /// + /// Invoked when a new non-null target is set + /// + /// New target + /// Result of newTarget.GetType() + protected internal virtual void OnNewNonNullTarget(object newTarget, Type newTargetType) { } + + /// + /// Invoked when a new non-null target is set, which has non-null MethodInfo. Used to assert that the method signature is correct + /// + /// MethodInfo of method on new target + /// Type of new target + protected internal abstract void AssertTargetMethodInfo(MethodInfo targetMethodInfo, Type newTargetType); + + /// + /// Invoked when a new target is set, after all other action has been taken + /// + /// Previous target + /// New target + protected internal virtual void OnTargetChanged(object oldTarget, object newTarget) { } + } +} diff --git a/Stylet/Xaml/CommandAction.cs b/Stylet/Xaml/CommandAction.cs index d75b65a..969f081 100644 --- a/Stylet/Xaml/CommandAction.cs +++ b/Stylet/Xaml/CommandAction.cs @@ -4,6 +4,7 @@ using System.ComponentModel; using System.Reflection; using System.Runtime.ExceptionServices; using System.Windows; +using System.Windows.Data; using System.Windows.Input; using Expressions = System.Linq.Expressions; @@ -17,35 +18,15 @@ namespace Stylet.Xaml /// Watches the current View.ActionTarget, and looks for a method with the given name, calling it when the ICommand is called. /// If a bool property with name Get(methodName) exists, it will be observed and used to enable/disable the ICommand. /// - public class CommandAction : ICommand + public class CommandAction : ActionBase, ICommand { private static readonly ILogger logger = LogManager.GetLogger(typeof(CommandAction)); - /// - /// Gets the View to grab the View.ActionTarget from - /// - public DependencyObject Subject { get; private set; } - - /// - /// Gets the method name. E.g. if someone's gone Buttom Command="{s:Action MyMethod}", this is MyMethod. - /// - public string MethodName { get; private set; } - /// /// Generated accessor to grab the value of the guard property, or null if there is none /// private Func guardPropertyGetter; - /// - /// MethodInfo for the method to call. This has to exist, or we throw a wobbly - /// - private MethodInfo targetMethodInfo; - - private object target; - - private readonly ActionUnavailableBehaviour targetNullBehaviour; - private readonly ActionUnavailableBehaviour actionNonExistentBehaviour; - /// /// Initialises a new instance of the class /// @@ -54,109 +35,68 @@ namespace Stylet.Xaml /// Behaviour for it the relevant View.ActionTarget is null /// Behaviour for if the action doesn't exist on the View.ActionTarget public CommandAction(DependencyObject subject, string methodName, ActionUnavailableBehaviour targetNullBehaviour, ActionUnavailableBehaviour actionNonExistentBehaviour) - { - this.Subject = subject; - this.MethodName = methodName; - this.targetNullBehaviour = targetNullBehaviour; - this.actionNonExistentBehaviour = actionNonExistentBehaviour; - - this.UpdateGuardAndMethod(); - - // Observe the View.ActionTarget for changes, and re-bind the guard property and MethodInfo if it changes - DependencyPropertyChangeNotifier.AddValueChanged(this.Subject, View.ActionTargetProperty, (o, e) => this.UpdateGuardAndMethod()); - } + : base(subject, methodName, targetNullBehaviour, actionNonExistentBehaviour, logger) + { } private string GuardName { get { return "Can" + this.MethodName; } } - private void UpdateGuardAndMethod() + /// + /// Invoked when a new non-null target is set + /// + /// New target + /// Result of newTarget.GetType() + protected internal override void OnNewNonNullTarget(object newTarget, Type newTargetType) { - var newTarget = View.GetActionTarget(this.Subject); - MethodInfo targetMethodInfo = null; - - // If it's being set to the initial value, ignore it - // At this point, we're executing the View's InitializeComponent method, and the ActionTarget hasn't yet been assigned - // If they've opted to throw if the target is null, then this will cause that exception. - // We'll just wait until the ActionTarget is assigned, and we're called again - if (newTarget == View.InitialActionTarget) + var guardPropertyInfo = newTargetType.GetProperty(this.GuardName); + if (guardPropertyInfo != null) { - this.target = newTarget; - return; - } - - this.guardPropertyGetter = null; - if (newTarget == null) - { - // If it's Enable or Disable we don't do anything - CanExecute will handle this - if (this.targetNullBehaviour == ActionUnavailableBehaviour.Throw) + if (guardPropertyInfo.PropertyType == typeof(bool)) { - var e = new ActionTargetNullException(String.Format("ActionTarget on element {0} is null (method name is {1})", this.Subject, this.MethodName)); - logger.Error(e); - throw e; + var targetExpression = Expressions.Expression.Constant(newTarget); + var propertyAccess = Expressions.Expression.Property(targetExpression, guardPropertyInfo); + this.guardPropertyGetter = Expressions.Expression.Lambda>(propertyAccess).Compile(); } else { - logger.Info("ActionTarget on element {0} is null (method name is {1}), but NullTarget is not Throw, so carrying on", this.Subject, this.MethodName); + logger.Warn("Found guard property {0} for action {1} on target {2}, but its return type wasn't bool. Therefore, ignoring", this.GuardName, this.MethodName, newTarget); } } - else + } + + /// + /// Invoked when a new non-null target is set, which has non-null MethodInfo. Used to assert that the method signature is correct + /// + /// MethodInfo of method on new target + /// Type of new target + protected internal override void AssertTargetMethodInfo(MethodInfo targetMethodInfo, Type newTargetType) + { + var methodParameters = targetMethodInfo.GetParameters(); + if (methodParameters.Length > 1) { - var newTargetType = newTarget.GetType(); - - var guardPropertyInfo = newTargetType.GetProperty(this.GuardName); - if (guardPropertyInfo != null) - { - if (guardPropertyInfo.PropertyType == typeof(bool)) - { - var targetExpression = Expressions.Expression.Constant(newTarget); - var propertyAccess = Expressions.Expression.Property(targetExpression, guardPropertyInfo); - this.guardPropertyGetter = Expressions.Expression.Lambda>(propertyAccess).Compile(); - } - else - { - logger.Warn("Found guard property {0} for action {1} on target {2}, but its return type wasn't bool. Therefore, ignoring", this.GuardName, this.MethodName, newTarget); - } - } - - targetMethodInfo = newTargetType.GetMethod(this.MethodName); - if (targetMethodInfo == null) - { - if (this.actionNonExistentBehaviour == ActionUnavailableBehaviour.Throw) - { - var e = new ActionNotFoundException(String.Format("Unable to find method {0} on {1}", this.MethodName, newTargetType.Name)); - logger.Error(e); - throw e; - } - else - { - logger.Warn("Unable to find method {0} on {1}, but ActionNotFound is not Throw, so carrying on", this.MethodName, newTargetType.Name); - } - } - else - { - var methodParameters = targetMethodInfo.GetParameters(); - if (methodParameters.Length > 1) - { - var e = new ActionSignatureInvalidException(String.Format("Method {0} on {1} must have zero or one parameters", this.MethodName, newTargetType.Name)); - logger.Error(e); - throw e; - } - } + var e = new ActionSignatureInvalidException(String.Format("Method {0} on {1} must have zero or one parameters", this.MethodName, newTargetType.Name)); + logger.Error(e); + throw e; } + } - var oldTarget = this.target as INotifyPropertyChanged; - if (oldTarget != null) - PropertyChangedEventManager.RemoveHandler(oldTarget, this.PropertyChangedHandler, this.GuardName); + /// + /// Invoked when a new target is set, after all other action has been taken + /// + /// Previous target + /// New target + protected internal override void OnTargetChanged(object oldTarget, object newTarget) + { + var oldInpc = oldTarget as INotifyPropertyChanged; + if (oldInpc != null) + PropertyChangedEventManager.RemoveHandler(oldInpc, this.PropertyChangedHandler, this.GuardName); var inpc = newTarget as INotifyPropertyChanged; if (this.guardPropertyGetter != null && inpc != null) PropertyChangedEventManager.AddHandler(inpc, this.PropertyChangedHandler, this.GuardName); - this.target = newTarget; - this.targetMethodInfo = targetMethodInfo; - this.UpdateCanExecute(); } @@ -186,19 +126,19 @@ namespace Stylet.Xaml // e.g. if the button/etc in question is in a ContextMenu/Popup, which attached properties // aren't inherited by. // Show the control as enabled, but throw if they try and click on it - if (this.target == View.InitialActionTarget) + if (this.Target == View.InitialActionTarget) return true; // It's enabled only if both the targetNull and actionNonExistent tests pass // Throw is handled when the target is set - if (this.target == null) - return this.targetNullBehaviour != ActionUnavailableBehaviour.Disable; + if (this.Target == null) + return this.TargetNullBehaviour != ActionUnavailableBehaviour.Disable; // Throw is handled when the target is set - if (this.targetMethodInfo == null) + if (this.TargetMethodInfo == null) { - if (this.actionNonExistentBehaviour == ActionUnavailableBehaviour.Disable) + if (this.ActionNonExistentBehaviour == ActionUnavailableBehaviour.Disable) return false; else return true; @@ -223,7 +163,7 @@ namespace Stylet.Xaml { // If we've made it this far and the target is still the default, then something's wrong // Make sure they know - if (this.target == View.InitialActionTarget) + if (this.Target == View.InitialActionTarget) { var e = new ActionNotSetException(String.Format("View.ActionTarget not on control {0} (method {1}). " + "This probably means the control hasn't inherited it from a parent, e.g. because a ContextMenu or Popup sits in the visual tree. " + @@ -233,22 +173,22 @@ namespace Stylet.Xaml } // Any throwing would have been handled prior to this - if (this.target == null || this.targetMethodInfo == null) + if (this.Target == null || this.TargetMethodInfo == null) return; // This is not going to be called very often, so don't bother to generate a delegate, in the way that we do for the method guard - var parameters = this.targetMethodInfo.GetParameters().Length == 1 ? new[] { parameter } : null; - logger.Info("Invoking method {0} on target {1} with parameters ({2})", this.MethodName, this.target, parameters == null ? "none" : String.Join(", ", parameters)); + var parameters = this.TargetMethodInfo.GetParameters().Length == 1 ? new[] { parameter } : null; + logger.Info("Invoking method {0} on target {1} with parameters ({2})", this.MethodName, this.Target, parameters == null ? "none" : String.Join(", ", parameters)); try { - this.targetMethodInfo.Invoke(this.target, parameters); + this.TargetMethodInfo.Invoke(this.Target, parameters); } catch (TargetInvocationException e) { // Be nice and unwrap this for them // They want a stack track for their VM method, not us - logger.Error(e.InnerException, String.Format("Failed to invoke method {0} on target {1} with parameters ({2})", this.MethodName, this.target, parameters == null ? "none" : String.Join(", ", parameters))); + logger.Error(e.InnerException, String.Format("Failed to invoke method {0} on target {1} with parameters ({2})", this.MethodName, this.Target, parameters == null ? "none" : String.Join(", ", parameters))); // http://stackoverflow.com/a/17091351/1086121 ExceptionDispatchInfo.Capture(e.InnerException).Throw(); } diff --git a/Stylet/Xaml/EventAction.cs b/Stylet/Xaml/EventAction.cs index 05ef5f3..e3c59d6 100644 --- a/Stylet/Xaml/EventAction.cs +++ b/Stylet/Xaml/EventAction.cs @@ -4,42 +4,23 @@ using System.ComponentModel; using System.Reflection; using System.Runtime.ExceptionServices; using System.Windows; +using System.Windows.Data; namespace Stylet.Xaml { /// /// Created by ActionExtension, this can return a delegate suitable adding binding to an event, and can call a method on the View.ActionTarget /// - public class EventAction + public class EventAction : ActionBase { private static readonly ILogger logger = LogManager.GetLogger(typeof(EventAction)); private static readonly MethodInfo invokeCommandMethodInfo = typeof(EventAction).GetMethod("InvokeCommand", BindingFlags.NonPublic | BindingFlags.Instance); - private readonly ActionUnavailableBehaviour targetNullBehaviour; - private readonly ActionUnavailableBehaviour actionNonExistentBehaviour; - - /// - /// View whose View.ActionTarget we watch - /// - private readonly DependencyObject subject; - - /// - /// The MyMethod in {s:Action MyMethod}, this is what we call when the event's fired - /// - private readonly string methodName; - /// /// Type of event handler /// private readonly Type eventHandlerType; - /// - /// MethodInfo for the method to call. This has to exist, or we throw a wobbly - /// - private MethodInfo targetMethodInfo; - - private object target; - /// /// Initialises a new instance of the class /// @@ -49,85 +30,32 @@ namespace Stylet.Xaml /// Behaviour for it the relevant View.ActionTarget is null /// Behaviour for if the action doesn't exist on the View.ActionTarget public EventAction(DependencyObject subject, Type eventHandlerType, string methodName, ActionUnavailableBehaviour targetNullBehaviour, ActionUnavailableBehaviour actionNonExistentBehaviour) + : base(subject, methodName, targetNullBehaviour, actionNonExistentBehaviour, logger) { if (targetNullBehaviour == ActionUnavailableBehaviour.Disable) throw new ArgumentException("Setting NullTarget = Disable is unsupported when used on an Event"); if (actionNonExistentBehaviour == ActionUnavailableBehaviour.Disable) throw new ArgumentException("Setting ActionNotFound = Disable is unsupported when used on an Event"); - this.subject = subject; this.eventHandlerType = eventHandlerType; - this.methodName = methodName; - this.targetNullBehaviour = targetNullBehaviour; - this.actionNonExistentBehaviour = actionNonExistentBehaviour; - - this.UpdateMethod(); - - // Observe the View.ActionTarget for changes, and re-bind the guard property and MethodInfo if it changes - DependencyPropertyChangeNotifier.AddValueChanged(this.subject, View.ActionTargetProperty, (o, e) => this.UpdateMethod()); } - private void UpdateMethod() + /// + /// Invoked when a new non-null target is set, which has non-null MethodInfo. Used to assert that the method signature is correct + /// + /// MethodInfo of method on new target + /// Type of new target + protected internal override void AssertTargetMethodInfo(MethodInfo targetMethodInfo, Type newTargetType) { - var newTarget = View.GetActionTarget(this.subject); - MethodInfo targetMethodInfo = null; - - // If it's being set to the initial value, ignore it - // At this point, we're executing the View's InitializeComponent method, and the ActionTarget hasn't yet been assigned - // If they've opted to throw if the target is null, then this will cause that exception. - // We'll just wait until the ActionTarget is assigned, and we're called again - if (newTarget == View.InitialActionTarget) + var methodParameters = targetMethodInfo.GetParameters(); + if (!(methodParameters.Length == 0 || + (methodParameters.Length == 1 && typeof(EventArgs).IsAssignableFrom(methodParameters[0].ParameterType)) || + (methodParameters.Length == 2 && typeof(EventArgs).IsAssignableFrom(methodParameters[1].ParameterType)))) { - this.target = newTarget; - return; + var e = new ActionSignatureInvalidException(String.Format("Method {0} on {1} must have the signatures void Method(), void Method(EventArgsOrSubClass e), or void Method(object sender, EventArgsOrSubClass e)", this.MethodName, newTargetType.Name)); + logger.Error(e); + throw e; } - - if (newTarget == null) - { - if (this.targetNullBehaviour == ActionUnavailableBehaviour.Throw) - { - var e = new ActionTargetNullException(String.Format("ActionTarget on element {0} is null (method name is {1})", this.subject, this.methodName)); - logger.Error(e); - throw e; - } - else - { - logger.Info("ActionTarget on element {0} is null (method name is {1}), nut NullTarget is not Throw, so carrying on", this.subject, this.methodName); - } - } - else - { - var newTargetType = newTarget.GetType(); - targetMethodInfo = newTargetType.GetMethod(this.methodName); - if (targetMethodInfo == null) - { - if (this.actionNonExistentBehaviour == ActionUnavailableBehaviour.Throw) - { - var e = new ActionNotFoundException(String.Format("Unable to find method {0} on {1}", this.methodName, newTargetType.Name)); - logger.Error(e); - throw e; - } - else - { - logger.Warn("Unable to find method {0} on {1}, but ActionNotFound is not Throw, so carrying on", this.methodName, newTargetType.Name); - } - } - else - { - var methodParameters = targetMethodInfo.GetParameters(); - if (!(methodParameters.Length == 0 || - (methodParameters.Length == 1 && typeof(EventArgs).IsAssignableFrom(methodParameters[0].ParameterType)) || - (methodParameters.Length == 2 && typeof(EventArgs).IsAssignableFrom(methodParameters[1].ParameterType)))) - { - var e = new ActionSignatureInvalidException(String.Format("Method {0} on {1} must have the signatures void Method(), void Method(EventArgsOrSubClass e), or void Method(object sender, EventArgsOrSubClass e)", this.methodName, newTargetType.Name)); - logger.Error(e); - throw e; - } - } - } - - this.target = newTarget; - this.targetMethodInfo = targetMethodInfo; } /// @@ -139,7 +67,7 @@ namespace Stylet.Xaml var del = Delegate.CreateDelegate(this.eventHandlerType, this, invokeCommandMethodInfo, false); if (del == null) { - var e = new ActionEventSignatureInvalidException(String.Format("Event being bound to does not have the '(object sender, EventArgsOrSubclass e)' signature we were expecting. Method {0} on target {1}", this.methodName, this.target)); + var e = new ActionEventSignatureInvalidException(String.Format("Event being bound to does not have the '(object sender, EventArgsOrSubclass e)' signature we were expecting. Method {0} on target {1}", this.MethodName, this.Target)); logger.Error(e); throw e; } @@ -151,21 +79,21 @@ namespace Stylet.Xaml { // If we've made it this far and the target is still the default, then something's wrong // Make sure they know - if (this.target == View.InitialActionTarget) + if (this.Target == View.InitialActionTarget) { var ex = new ActionNotSetException(String.Format("View.ActionTarget not on control {0} (method {1}). " + "This probably means the control hasn't inherited it from a parent, e.g. because a ContextMenu or Popup sits in the visual tree. " + - "You will need so set 's:View.ActionTarget' explicitly. See the wiki for more details.", this.subject, this.methodName)); + "You will need so set 's:View.ActionTarget' explicitly. See the wiki for more details.", this.Subject, this.MethodName)); logger.Error(ex); throw ex; } // Any throwing will have been handled above - if (this.target == null || this.targetMethodInfo == null) + if (this.Target == null || this.TargetMethodInfo == null) return; object[] parameters; - switch (this.targetMethodInfo.GetParameters().Length) + switch (this.TargetMethodInfo.GetParameters().Length) { case 1: parameters = new object[] { e }; @@ -180,17 +108,17 @@ namespace Stylet.Xaml break; } - logger.Info("Invoking method {0} on target {1} with parameters ({2})", this.methodName, this.target, parameters == null ? "none" : String.Join(", ", parameters)); + logger.Info("Invoking method {0} on target {1} with parameters ({2})", this.MethodName, this.Target, parameters == null ? "none" : String.Join(", ", parameters)); try { - this.targetMethodInfo.Invoke(this.target, parameters); + this.TargetMethodInfo.Invoke(this.Target, parameters); } catch (TargetInvocationException ex) { // Be nice and unwrap this for them // They want a stack track for their VM method, not us - logger.Error(ex.InnerException, String.Format("Failed to invoke method {0} on target {1} with parameters ({2})", this.methodName, this.target, parameters == null ? "none" : String.Join(", ", parameters))); + logger.Error(ex.InnerException, String.Format("Failed to invoke method {0} on target {1} with parameters ({2})", this.MethodName, this.Target, parameters == null ? "none" : String.Join(", ", parameters))); // http://stackoverflow.com/a/17091351/1086121 ExceptionDispatchInfo.Capture(ex.InnerException).Throw(); } diff --git a/StyletUnitTests/CommandActionTests.cs b/StyletUnitTests/CommandActionTests.cs index 7137232..90c14de 100644 --- a/StyletUnitTests/CommandActionTests.cs +++ b/StyletUnitTests/CommandActionTests.cs @@ -270,5 +270,19 @@ namespace StyletUnitTests Assert.False(weakView.IsAlive); } + + [Test] + public void OperatesAfterCollection() + { + var view = new DependencyObject(); + var cmd = new CommandAction(view, "DoSomething", ActionUnavailableBehaviour.Throw, ActionUnavailableBehaviour.Throw); + + GC.Collect(); + + View.SetActionTarget(view, this.target); + + cmd.Execute(null); + Assert.IsTrue(this.target.DoSomethingCalled); + } } } diff --git a/StyletUnitTests/DependencyPropertyChangeNotifierTests.cs b/StyletUnitTests/DependencyPropertyChangeNotifierTests.cs deleted file mode 100644 index 644e67b..0000000 --- a/StyletUnitTests/DependencyPropertyChangeNotifierTests.cs +++ /dev/null @@ -1,100 +0,0 @@ -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(() => DependencyPropertyChangeNotifier.AddValueChanged(null, View.ActionTargetProperty, (d, e) => { })); - } - - [Test] - public void ThrowsIfPropertyIsNull() - { - Assert.Throws(() => DependencyPropertyChangeNotifier.AddValueChanged(new DependencyObject(), (PropertyPath)null, (d, e) => { })); - Assert.Throws(() => DependencyPropertyChangeNotifier.AddValueChanged(new DependencyObject(), (DependencyProperty)null, (d, e) => { })); - } - - [Test] - public void ThrowsIfHandlerIsNull() - { - Assert.Throws(() => 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); - } - } -} diff --git a/StyletUnitTests/EventActionTests.cs b/StyletUnitTests/EventActionTests.cs index 2660b6d..0c0fc4e 100644 --- a/StyletUnitTests/EventActionTests.cs +++ b/StyletUnitTests/EventActionTests.cs @@ -214,5 +214,19 @@ namespace StyletUnitTests Assert.IsFalse(weakView.IsAlive); } + + [Test] + public void OperatesAfterCollection() + { + var view = new DependencyObject(); + var cmd = new EventAction(view, this.eventInfo.EventHandlerType, "DoSomething", ActionUnavailableBehaviour.Throw, ActionUnavailableBehaviour.Throw); + + GC.Collect(); + + View.SetActionTarget(view, this.target); + + cmd.GetDelegate().DynamicInvoke(null, null); + Assert.IsTrue(this.target.DoSomethingCalled); + } } } diff --git a/StyletUnitTests/StyletUnitTests.csproj b/StyletUnitTests/StyletUnitTests.csproj index fb29425..fca494c 100644 --- a/StyletUnitTests/StyletUnitTests.csproj +++ b/StyletUnitTests/StyletUnitTests.csproj @@ -64,7 +64,6 @@ -