diff options
author | E.Z. Hart <hartez@users.noreply.github.com> | 2016-12-01 14:15:17 -0700 |
---|---|---|
committer | Stephane Delcroix <stephane@delcroix.org> | 2016-12-01 22:15:17 +0100 |
commit | 46c25a2edbf257153d7bb33d2ac3997a3718e3ee (patch) | |
tree | 92209a25a152c11af33684c1e1933514ac4c407a | |
parent | 3d85653f270854b4aab82e45e6e58afb760feb85 (diff) | |
download | xamarin-forms-46c25a2edbf257153d7bb33d2ac3997a3718e3ee.tar.gz xamarin-forms-46c25a2edbf257153d7bb33d2ac3997a3718e3ee.tar.bz2 xamarin-forms-46c25a2edbf257153d7bb33d2ac3997a3718e3ee.zip |
Don't run Command CanExecute on incorrect inherited binding context type (#572)
* Allow Command CanExecute to recover when run on inherited bindingcontext
* Make exception handler more generic
* Checking types in Command delegates to avoid exception in the first place
* Adding type chekc to other Command constructor
* Use nameof for ArgumentNullExceptions
* Add unit tests for null parameters, handle value types and Nullable<T>
4 files changed, 258 insertions, 14 deletions
diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla47971.cs b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla47971.cs new file mode 100644 index 00000000..351028d4 --- /dev/null +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Bugzilla47971.cs @@ -0,0 +1,109 @@ +using System; +using System.Collections.Generic; +using System.Collections.ObjectModel; +using System.ComponentModel; +using System.Runtime.CompilerServices; +using Xamarin.Forms.CustomAttributes; +using Xamarin.Forms.Internals; + +namespace Xamarin.Forms.Controls.Issues +{ + [Preserve(AllMembers = true)] + [Issue(IssueTracker.Bugzilla, 47971, "UWP doesn't display list items when binding a CommandParameter to BindingContext in a DataTemplate and including a CanExecute method", PlatformAffected.WinRT)] + public class Bugzilla47971 : TestContentPage + { + protected override void Init() + { + var viewModel = new _47971ViewModel(); + + var lv = new ListView { BindingContext = viewModel }; + + lv.SetBinding(ListView.ItemsSourceProperty, new Binding("Models")); + lv.SetBinding(ListView.SelectedItemProperty, new Binding("SelectedModel")); + + lv.ItemTemplate = new DataTemplate(() => + { + var tc = new TextCell(); + + tc.SetBinding(TextCell.TextProperty, new Binding("Name")); + tc.SetBinding(TextCell.CommandParameterProperty, new Binding(".")); + tc.SetBinding(TextCell.CommandProperty, new Binding("BindingContext.ModelSelectedCommand", source: lv)); + + return tc; + }); + + var layout = new StackLayout { Spacing = 10 }; + var instructions = new Label {Text = "The ListView below should display three items (Item1, Item2, and Item3). If it does not, this test has failed." }; + + layout.Children.Add(instructions); + layout.Children.Add(lv); + + Content = layout; + } + + [Preserve(AllMembers = true)] + internal class _47971ViewModel : INotifyPropertyChanged + { + _47971ItemModel _selectedModel; + Command<_47971ItemModel> _modelSelectedCommand; + ObservableCollection<_47971ItemModel> _models; + + public ObservableCollection<_47971ItemModel> Models + { + get { return _models; } + set + { + _models = value; + OnPropertyChanged(); + } + } + + public _47971ItemModel SelectedModel + { + get { return _selectedModel; } + set + { + _selectedModel = value; + OnPropertyChanged(); + } + } + + public Command<_47971ItemModel> ModelSelectedCommand => _modelSelectedCommand ?? + (_modelSelectedCommand = new Command<_47971ItemModel>(ModelSelectedCommandExecute, CanExecute)); + + bool CanExecute(_47971ItemModel itemModel) + { + return true; + } + + void ModelSelectedCommandExecute(_47971ItemModel model) + { + System.Diagnostics.Debug.WriteLine(model.Name); + } + + public _47971ViewModel() + { + _models = new ObservableCollection<_47971ItemModel>( + new List<_47971ItemModel>() + { + new _47971ItemModel() { Name = "Item 1"}, + new _47971ItemModel() { Name = "Item 2"}, + new _47971ItemModel() { Name = "Item 3"} + }); + } + + public event PropertyChangedEventHandler PropertyChanged; + + protected virtual void OnPropertyChanged([CallerMemberName] string propertyName = null) + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName)); + } + } + + [Preserve(AllMembers = true)] + internal class _47971ItemModel + { + public string Name { get; set; } + } + } +}
\ No newline at end of file diff --git a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems index 3b728f4d..08c26ba1 100644 --- a/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems +++ b/Xamarin.Forms.Controls.Issues/Xamarin.Forms.Controls.Issues.Shared/Xamarin.Forms.Controls.Issues.Shared.projitems @@ -139,6 +139,7 @@ <Compile Include="$(MSBuildThisFileDirectory)Bugzilla46494.cs" /> <Compile Include="$(MSBuildThisFileDirectory)Bugzilla44476.cs" /> <Compile Include="$(MSBuildThisFileDirectory)Bugzilla46630.cs" /> + <Compile Include="$(MSBuildThisFileDirectory)Bugzilla47971.cs" /> <Compile Include="$(MSBuildThisFileDirectory)CarouselAsync.cs" /> <Compile Include="$(MSBuildThisFileDirectory)Bugzilla34561.cs" /> <Compile Include="$(MSBuildThisFileDirectory)Bugzilla34727.cs" /> diff --git a/Xamarin.Forms.Core.UnitTests/CommandTests.cs b/Xamarin.Forms.Core.UnitTests/CommandTests.cs index 1182fe40..c653b976 100644 --- a/Xamarin.Forms.Core.UnitTests/CommandTests.cs +++ b/Xamarin.Forms.Core.UnitTests/CommandTests.cs @@ -1,7 +1,6 @@ using System; using NUnit.Framework; - namespace Xamarin.Forms.Core.UnitTests { [TestFixture] @@ -151,5 +150,104 @@ namespace Xamarin.Forms.Core.UnitTests Assert.AreEqual (expected, cmd.CanExecute ("Foo")); Assert.AreEqual ("Foo", result); } + + class FakeParentContext + { + } + + // ReSharper disable once ClassNeverInstantiated.Local + class FakeChildContext + { + } + + [Test] + public void CanExecuteReturnsFalseIfParameterIsWrongReferenceType() + { + var command = new Command<FakeChildContext>(context => { }, context => true); + + Assert.IsFalse(command.CanExecute(new FakeParentContext()), "the parameter is of the wrong type"); + } + + [Test] + public void CanExecuteReturnsFalseIfParameterIsWrongValueType() + { + var command = new Command<int>(context => { }, context => true); + + Assert.IsFalse(command.CanExecute(10.5), "the parameter is of the wrong type"); + } + + [Test] + public void CanExecuteUsesParameterIfReferenceTypeAndSetToNull() + { + var command = new Command<FakeChildContext>(context => { }, context => true); + + Assert.IsTrue(command.CanExecute(null), "null is a valid value for a reference type"); + } + + [Test] + public void CanExecuteUsesParameterIfNullableAndSetToNull() + { + var command = new Command<int?>(context => { }, context => true); + + Assert.IsTrue(command.CanExecute(null), "null is a valid value for a Nullable<int> type"); + } + + [Test] + public void CanExecuteIgnoresParameterIfValueTypeAndSetToNull() + { + var command = new Command<int>(context => { }, context => true); + + Assert.IsFalse(command.CanExecute(null), "null is not a valid valid for int"); + } + + [Test] + public void ExecuteDoesNotRunIfParameterIsWrongReferenceType() + { + int executions = 0; + var command = new Command<FakeChildContext>(context => executions += 1); + + Assert.DoesNotThrow(() => command.Execute(new FakeParentContext()), "the command should not execute, so no exception should be thrown"); + Assert.IsTrue(executions == 0, "the command should not have executed"); + } + + [Test] + public void ExecuteDoesNotRunIfParameterIsWrongValueType() + { + int executions = 0; + var command = new Command<int>(context => executions += 1); + + Assert.DoesNotThrow(() => command.Execute(10.5), "the command should not execute, so no exception should be thrown"); + Assert.IsTrue(executions == 0, "the command should not have executed"); + } + + [Test] + public void ExecuteRunsIfReferenceTypeAndSetToNull() + { + int executions = 0; + var command = new Command<FakeChildContext>(context => executions += 1); + + Assert.DoesNotThrow(() => command.Execute(null), "null is a valid value for a reference type"); + Assert.IsTrue(executions == 1, "the command should have executed"); + } + + [Test] + public void ExecuteRunsIfNullableAndSetToNull() + { + int executions = 0; + var command = new Command<int?>(context => executions += 1); + + Assert.DoesNotThrow(() => command.Execute(null), "null is a valid value for a Nullable<int> type"); + Assert.IsTrue(executions == 1, "the command should have executed"); + } + + [Test] + public void ExecuteDoesNotRunIfValueTypeAndSetToNull() + { + int executions = 0; + var command = new Command<int>(context => executions += 1); + + Assert.DoesNotThrow(() => command.Execute(null), "null is not a valid value for int"); + Assert.IsTrue(executions == 0, "the command should not have executed"); + } } } diff --git a/Xamarin.Forms.Core/Command.cs b/Xamarin.Forms.Core/Command.cs index 73ae1b08..d15574cf 100644 --- a/Xamarin.Forms.Core/Command.cs +++ b/Xamarin.Forms.Core/Command.cs @@ -1,22 +1,59 @@ using System; +using System.Reflection; using System.Windows.Input; namespace Xamarin.Forms { - public sealed class Command<T> : Command + public sealed class Command<T> : Command { - public Command(Action<T> execute) : base(o => execute((T)o)) + public Command(Action<T> execute) + : base(o => + { + if (IsValidParameter(o)) + { + execute((T)o); + } + }) { if (execute == null) - throw new ArgumentNullException("execute"); + { + throw new ArgumentNullException(nameof(execute)); + } } - public Command(Action<T> execute, Func<T, bool> canExecute) : base(o => execute((T)o), o => canExecute((T)o)) + public Command(Action<T> execute, Func<T, bool> canExecute) + : base(o => + { + if (IsValidParameter(o)) + { + execute((T)o); + } + }, o => IsValidParameter(o) && canExecute((T)o)) { if (execute == null) - throw new ArgumentNullException("execute"); + throw new ArgumentNullException(nameof(execute)); if (canExecute == null) - throw new ArgumentNullException("canExecute"); + throw new ArgumentNullException(nameof(canExecute)); + } + + static bool IsValidParameter(object o) + { + if (o != null) + { + // The parameter isn't null, so we don't have to worry whether null is a valid option + return o is T; + } + + var t = typeof(T); + + // The parameter is null. Is T Nullable? + if (Nullable.GetUnderlyingType(t) != null) + { + return true; + } + + // Not a Nullable, if it's a value type then null is not valid + return !t.GetTypeInfo().IsValueType; } } @@ -28,7 +65,7 @@ namespace Xamarin.Forms public Command(Action<object> execute) { if (execute == null) - throw new ArgumentNullException("execute"); + throw new ArgumentNullException(nameof(execute)); _execute = execute; } @@ -36,13 +73,13 @@ namespace Xamarin.Forms public Command(Action execute) : this(o => execute()) { if (execute == null) - throw new ArgumentNullException("execute"); + throw new ArgumentNullException(nameof(execute)); } public Command(Action<object> execute, Func<object, bool> canExecute) : this(execute) { if (canExecute == null) - throw new ArgumentNullException("canExecute"); + throw new ArgumentNullException(nameof(canExecute)); _canExecute = canExecute; } @@ -50,9 +87,9 @@ namespace Xamarin.Forms public Command(Action execute, Func<bool> canExecute) : this(o => execute(), o => canExecute()) { if (execute == null) - throw new ArgumentNullException("execute"); + throw new ArgumentNullException(nameof(execute)); if (canExecute == null) - throw new ArgumentNullException("canExecute"); + throw new ArgumentNullException(nameof(canExecute)); } public bool CanExecute(object parameter) @@ -73,8 +110,7 @@ namespace Xamarin.Forms public void ChangeCanExecute() { EventHandler changed = CanExecuteChanged; - if (changed != null) - changed(this, EventArgs.Empty); + changed?.Invoke(this, EventArgs.Empty); } } }
\ No newline at end of file |