AMT-Cheif
/
Stylet
mirror of https://github.com/AMT-Cheif/Stylet.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Only throw on click if action not found 

It turns out there was a lovely little race with View.ActionTarget, where
either the View.ActionTarget or the corresponding {s:Action} could be
evaluated first. This meant setting View.ActionTarget was risky.

It happened to work in previous releases because the bootstrapper accessed
View.ViewManager, which caused its class ctor to run and somehow caused
View.ActionTarget to be evaluated earlier.

I don't think there's a decent way of getting around this: we can't
guarentee the order in which things in Xaml are going to be evaluated.
That's not the WPF way.

Therefore, only throw exceptions if someone actually clicks the button.
This commit is contained in:
Antony Male 2015-10-03 15:50:53 +01:00
parent 9263753600
commit 447f68f1f0
7 changed files with 58 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

34
Stylet/Xaml/ActionBase.cs
View File

@ -93,9 +93,7 @@ namespace Stylet.Xaml
// 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)
{
@ -114,28 +112,12 @@ namespace Stylet.Xaml
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);
}
}
this.logger.Warn("Unable to find method {0} on {1}", this.MethodName, newTargetType.Name);
else
{
this.AssertTargetMethodInfo(targetMethodInfo, newTargetType);
}
}
this.TargetMethodInfo = targetMethodInfo;
@ -143,13 +125,6 @@ namespace Stylet.Xaml
this.OnTargetChanged(oldTarget, newTarget);
}
/// <summary>
/// Invoked when a new non-null target is set
/// </summary>
/// <param name="newTarget">New target</param>
/// <param name="newTargetType">Result of newTarget.GetType()</param>
protected internal virtual void OnNewNonNullTarget(object newTarget, Type newTargetType) { }
/// <summary>
/// Invoked when a new non-null target is set, which has non-null MethodInfo. Used to assert that the method signature is correct
/// </summary>
@ -179,6 +154,13 @@ namespace Stylet.Xaml
this.logger.Error(ex);
throw ex;
}
if (this.TargetMethodInfo == null && this.ActionNonExistentBehaviour == ActionUnavailableBehaviour.Throw)
{
var ex = new ActionNotFoundException(String.Format("Unable to find method {0} on target {1}", this.MethodName, this.Target.GetType().Name));
logger.Error(ex);
throw ex;
}
}
/// <summary>

2
Stylet/Xaml/ActionExtension.cs
View File

@ -27,7 +27,7 @@ namespace Stylet.Xaml
Disable,
/// <summary>
/// Throw an exception
/// An exception will be thrown when the control is clicked
/// </summary>
Throw
}

47
Stylet/Xaml/CommandAction.cs
View File

@ -41,29 +41,6 @@ namespace Stylet.Xaml
get { return "Can" + this.MethodName; }
}
/// <summary>
/// Invoked when a new non-null target is set
/// </summary>
/// <param name="newTarget">New target</param>
/// <param name="newTargetType">Result of newTarget.GetType()</param>
protected internal override void OnNewNonNullTarget(object newTarget, Type newTargetType)
{
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<Func<bool>>(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);
}
}
}
/// <summary>
/// Invoked when a new non-null target is set, which has non-null MethodInfo. Used to assert that the method signature is correct
/// </summary>
@ -91,9 +68,29 @@ namespace Stylet.Xaml
if (oldInpc != null)
PropertyChangedEventManager.RemoveHandler(oldInpc, this.PropertyChangedHandler, this.GuardName);
this.guardPropertyGetter = null;
var inpc = newTarget as INotifyPropertyChanged;
if (this.guardPropertyGetter != null && inpc != null)
PropertyChangedEventManager.AddHandler(inpc, this.PropertyChangedHandler, this.GuardName);
if (inpc != null)
{
var guardPropertyInfo = newTarget.GetType().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<Func<bool>>(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);
}
}
if (this.guardPropertyGetter != null)
PropertyChangedEventManager.AddHandler(inpc, this.PropertyChangedHandler, this.GuardName);
}
this.UpdateCanExecute();
}

1
StyletIntegrationTests/ShellView.xaml
View File

@ -45,6 +45,7 @@
</ContextMenu>
</TextBox.ContextMenu>
</TextBox>
<Button DockPanel.Dock="Top" s:View.ActionTarget="{Binding ChildViewModel}" Command="{s:Action Foo}">Click Me</Button>
</DockPanel>
</GroupBox>

21
StyletIntegrationTests/ShellViewModel.cs
View File

@ -10,16 +10,20 @@ namespace StyletIntegrationTests
{
public class ShellViewModel : Screen
{
private IWindowManager windowManager;
private readonly IWindowManager windowManager;
public ChildViewModel ChildViewModel { get; private set; }
public ShellViewModel(IWindowManager windowManager)
{
this.windowManager = windowManager;
this.ChildViewModel = new ChildViewModel(windowManager);
this.DisplayName = "ShellViewModel";
}
private bool? _showDialogAndDialogResultDialogResult;
public bool? ShowDialogAndDialogResultDialogResult
{
@ -75,4 +79,19 @@ namespace StyletIntegrationTests
get { return "Pass"; }
}
}
public class ChildViewModel
{
private readonly IWindowManager windowManager;
public ChildViewModel(IWindowManager windowManager)
{
this.windowManager = windowManager;
}
public void Foo()
{
this.windowManager.ShowMessageBox("Success!");
}
}
}

3
StyletUnitTests/CommandActionTests.cs
View File

@ -100,7 +100,8 @@ namespace StyletUnitTests
public void ThrowsIfActionNonExistentBehaviourIsThrowAndActionIsNonExistent()
{
var cmd = new CommandAction(this.subject, "DoSomething", ActionUnavailableBehaviour.Throw, ActionUnavailableBehaviour.Throw);
Assert.Throws<ActionNotFoundException>(() => View.SetActionTarget(this.subject, new Target2()));
Assert.DoesNotThrow(() => View.SetActionTarget(this.subject, new Target2()));
Assert.Throws<ActionNotFoundException>(() => cmd.Execute(null));
}
[Test]

6
StyletUnitTests/EventActionTests.cs
View File

@ -109,10 +109,12 @@ namespace StyletUnitTests
}
[Test]
public void ThrowsIfActionNonExistentBehaviourIsThrowAndActionIsNonExistent()
public void ThrowsWhenClickedIfActionNonExistentBehaviourIsThrowAndActionIsNonExistent()
{
var cmd = new EventAction(this.subject, this.eventInfo.EventHandlerType, "DoSomething", ActionUnavailableBehaviour.Enable, ActionUnavailableBehaviour.Throw);
Assert.Throws<ActionNotFoundException>(() => View.SetActionTarget(this.subject, new Target2()));
Assert.DoesNotThrow(() => View.SetActionTarget(this.subject, new Target2()));
var e = Assert.Throws<TargetInvocationException>(() => cmd.GetDelegate().DynamicInvoke(null, new RoutedEventArgs()));
Assert.IsInstanceOf<ActionNotFoundException>(e.InnerException);
}
[Test]