Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validations system refactor. #2223

Merged
merged 24 commits into from Sep 10, 2019
Merged
Changes from 4 commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -329,7 +329,7 @@ public override void OnDeselected()
Global.ChaumianClient.DeactivateFrequentStatusProcessingIfNotMixing();
}

public string ValidatePassword() => PasswordHelper.ValidatePassword(Password);
public ErrorDescriptors ValidatePassword() => PasswordHelper.ValidatePassword(Password);

[ValidateMethod(nameof(ValidatePassword))]
public string Password
@@ -966,7 +966,7 @@ public Money AllSelectedAmount
set => this.RaiseAndSetIfChanged(ref _allSelectedAmount, value);
}

public string ValidatePassword() => PasswordHelper.ValidatePassword(Password);
public ErrorDescriptors ValidatePassword() => PasswordHelper.ValidatePassword(Password);

[ValidateMethod(nameof(ValidatePassword))]
public string Password
@@ -1035,24 +1035,24 @@ public void OnAddWord(string word)
Suggestions.Clear();
}

public string ValidateAddress()
public ErrorDescriptors ValidateAddress()
{
if (string.IsNullOrWhiteSpace(Address))
{
return "";
return ErrorDescriptors.Empty;
}

if (AddressStringParser.TryParseBitcoinAddress(Address, Global.Network, out _))
{
return "";
return ErrorDescriptors.Empty;
}

if (AddressStringParser.TryParseBitcoinUrl(Address, Global.Network, out _))
{
return "";
return ErrorDescriptors.Empty;
}

return "Invalid address.";
return new ErrorDescriptors(new ErrorDescriptor(ErrorSeverity.Warning, "Invalid address."));
}

[ValidateMethod(nameof(ValidateAddress))]
@@ -90,7 +90,7 @@ public bool ShowSensitiveKeys
set => this.RaiseAndSetIfChanged(ref _showSensitiveKeys, value);
}

public string ValidatePassword() => PasswordHelper.ValidatePassword(Password);
public ErrorDescriptors ValidatePassword() => PasswordHelper.ValidatePassword(Password);

[ValidateMethod(nameof(ValidatePassword))]
public string Password
@@ -287,12 +287,12 @@ private void Save()
}

var isValid =
string.IsNullOrEmpty(ValidatePrivacyLevel(SomePrivacyLevel, whiteSpaceOk: false))
&& string.IsNullOrEmpty(ValidatePrivacyLevel(FinePrivacyLevel, whiteSpaceOk: false))
&& string.IsNullOrEmpty(ValidatePrivacyLevel(StrongPrivacyLevel, whiteSpaceOk: false))
&& string.IsNullOrEmpty(ValidateDustThreshold(DustThreshold, whiteSpaceOk: false))
&& string.IsNullOrEmpty(ValidateEndPoint(TorSocks5EndPoint, Constants.DefaultTorSocksPort, whiteSpaceOk: false))
&& string.IsNullOrEmpty(ValidateEndPoint(BitcoinP2pEndPoint, network.DefaultPort, whiteSpaceOk: false));
!ValidatePrivacyLevel(SomePrivacyLevel, whiteSpaceOk: false).HasErrors
&& !ValidatePrivacyLevel(FinePrivacyLevel, whiteSpaceOk: false).HasErrors
&& !ValidatePrivacyLevel(StrongPrivacyLevel, whiteSpaceOk: false).HasErrors
&& !ValidateDustThreshold(DustThreshold, whiteSpaceOk: false).HasErrors
&& !ValidateEndPoint(TorSocks5EndPoint, Constants.DefaultTorSocksPort, whiteSpaceOk: false).HasErrors
&& !ValidateEndPoint(BitcoinP2pEndPoint, network.DefaultPort, whiteSpaceOk: false).HasErrors;

if (!isValid)
{
@@ -340,67 +340,67 @@ private void OpenConfigFile()

#region Validation

public string ValidateSomePrivacyLevel()
public ErrorDescriptors ValidateSomePrivacyLevel()
=> ValidatePrivacyLevel(SomePrivacyLevel, whiteSpaceOk: true);

public string ValidateFinePrivacyLevel()
public ErrorDescriptors ValidateFinePrivacyLevel()
=> ValidatePrivacyLevel(FinePrivacyLevel, whiteSpaceOk: true);

public string ValidateStrongPrivacyLevel()
public ErrorDescriptors ValidateStrongPrivacyLevel()
=> ValidatePrivacyLevel(StrongPrivacyLevel, whiteSpaceOk: true);

public string ValidateDustThreshold()
public ErrorDescriptors ValidateDustThreshold()
=> ValidateDustThreshold(DustThreshold, whiteSpaceOk: true);

public string ValidateTorSocks5EndPoint()
public ErrorDescriptors ValidateTorSocks5EndPoint()
=> ValidateEndPoint(TorSocks5EndPoint, Constants.DefaultTorSocksPort, whiteSpaceOk: true);

public string ValidateBitcoinP2pEndPoint()
public ErrorDescriptors ValidateBitcoinP2pEndPoint()
=> ValidateEndPoint(BitcoinP2pEndPoint, Network.DefaultPort, whiteSpaceOk: true);

public string ValidatePrivacyLevel(string value, bool whiteSpaceOk)
public ErrorDescriptors ValidatePrivacyLevel(string value, bool whiteSpaceOk)
{
if (whiteSpaceOk && string.IsNullOrWhiteSpace(value))
{
return string.Empty;
return ErrorDescriptors.Empty;
}

if (uint.TryParse(value, out _))
{
return string.Empty;
return ErrorDescriptors.Empty;
}

return "Invalid privacy level.";
return new ErrorDescriptors(new ErrorDescriptor(ErrorSeverity.Warning, "Invalid privacy level."));
}

public string ValidateDustThreshold(string dustThreshold, bool whiteSpaceOk)
public ErrorDescriptors ValidateDustThreshold(string dustThreshold, bool whiteSpaceOk)
{
if (whiteSpaceOk && string.IsNullOrWhiteSpace(dustThreshold))
{
return string.Empty;
return ErrorDescriptors.Empty;
}

if (decimal.TryParse(dustThreshold, out var dust) && dust >= 0)
{
return string.Empty;
return ErrorDescriptors.Empty;
}

return "Invalid dust threshold.";
return new ErrorDescriptors(new ErrorDescriptor(ErrorSeverity.Warning, "Invalid dust threshold."));
}

public string ValidateEndPoint(string endPoint, int defaultPort, bool whiteSpaceOk)
public ErrorDescriptors ValidateEndPoint(string endPoint, int defaultPort, bool whiteSpaceOk)
{
if (whiteSpaceOk && string.IsNullOrWhiteSpace(endPoint))
{
return string.Empty;
return ErrorDescriptors.Empty;
}

if (EndPointParser.TryParse(endPoint, defaultPort, out _))
{
return string.Empty;
return ErrorDescriptors.Empty;
}

return "Invalid endpoint.";
return new ErrorDescriptors(new ErrorDescriptor(ErrorSeverity.Warning, "Invalid endpoint."));
}

#endregion Validation
@@ -31,7 +31,7 @@ public GenerateWalletViewModel(WalletManagerViewModel owner) : base("Generate Wa

IObservable<bool> canGenerate = Observable.CombineLatest(
this.WhenAnyValue(x => x.TermsAccepted),
this.WhenAnyValue(x => x.Password).Select(pw => string.IsNullOrEmpty(ValidatePassword())),
this.WhenAnyValue(x => x.Password).Select(pw => !ValidatePassword().HasErrors),
(terms, pw) => terms && pw);

GenerateCommand = ReactiveCommand.Create(DoGenerateCommand, canGenerate);
@@ -94,21 +94,25 @@ private bool ValidateWalletName(string walletName)
return isValid && !isReserved;
}

public string ValidatePassword()
public ErrorDescriptors ValidatePassword()
{
string password = Password;

List<string> messages = new List<string>();

var errors = new ErrorDescriptors();

if (PasswordHelper.IsTrimable(password, out _))
{
messages.Add("Leading and trailing white spaces are not allowed!");
errors.Add(new ErrorDescriptor(ErrorSeverity.Warning, "Leading and trailing white spaces are not allowed!"));
}

if (PasswordHelper.IsTooLong(password, out _))
{
messages.Add(PasswordHelper.PasswordTooLongMessage);
errors.Add(new ErrorDescriptor(ErrorSeverity.Warning, PasswordHelper.PasswordTooLongMessage));
}

return string.Join(' ', messages);
return errors;
}

[ValidateMethod(nameof(ValidatePassword))]
@@ -169,7 +169,7 @@ public ObservableCollection<LoadWalletEntry> Wallets
set => this.RaiseAndSetIfChanged(ref _wallets, value);
}

public string ValidatePassword() => PasswordHelper.ValidatePassword(Password);
public ErrorDescriptors ValidatePassword() => PasswordHelper.ValidatePassword(Password);

[ValidateMethod(nameof(ValidatePassword))]
public string Password
@@ -92,7 +92,7 @@ public RecoverWalletViewModel(WalletManagerViewModel owner) : base("Recover Wall
_suggestions = new ObservableCollection<SuggestionViewModel>();
}

public string ValidatePassword() => PasswordHelper.ValidatePassword(Password);
public ErrorDescriptors ValidatePassword() => PasswordHelper.ValidatePassword(Password);

[ValidateMethod(nameof(ValidatePassword))]
public string Password
@@ -101,6 +101,11 @@ public string Password
set => this.RaiseAndSetIfChanged(ref _password, value);
}

private void ValidatePasswordN(ErrorDescriptor obj)
This conversation was marked as resolved by jmacato

This comment has been minimized.

Copy link
@yahiheb

yahiheb Sep 9, 2019

Collaborator

This method is not used anywhere should it be removed?.
Why its name is ValidatePasswordN with N?

This comment has been minimized.

Copy link
@jmacato

jmacato Sep 9, 2019

Author Collaborator

Forgot to remove that sorry :)

{
throw new NotImplementedException();
}

public string MnemonicWords
{
get => _mnemonicWords;
@@ -50,6 +50,22 @@ public static T InvokeMethod<T>(object instance, string methodName)
}
}

public static MethodInfo GetMethodInfo<T>(object instance, string methodName)
{
MethodInfo info = instance.GetType().GetRuntimeMethod(methodName, new Type[0]);

if (info != null &&
info.ReturnType == typeof(T) &&
info.GetParameters().Length == 0)
{
return info;
}
else
{
throw new ArgumentException("Method was not found on class");
}
}

This conversation was marked as resolved by molnard

This comment has been minimized.

Copy link
@nopara73

nopara73 Sep 10, 2019

Collaborator

Reflection is generally to be avoided. Is this absolutely necessary?

This comment has been minimized.

Copy link
@jmacato

jmacato Sep 10, 2019

Author Collaborator

Yes, i think this, for a fact, reduces reflection calls because we only scan for methods via reflection on startup then the information about properties with validation attributes are cached into ViewModelBase. Instead of calling reflection to scan for properties every single validation call (like it was previously), we just use the cache to match property names to the MethodInfo that we saved.

public static IEnumerable<PropertyInfo> GetPropertyInfos(object instance)
{
return instance.GetType().GetRuntimeProperties();
@@ -1,50 +1,61 @@
using System;
using System.Collections.Generic;
using System.Reflection;
using System.Text;
using WalletWasabi.Helpers;

namespace WalletWasabi.Gui.ViewModels.Validation
{
public static class Validator
{
public static List<string> ValidateAllProperties(object instance)
public static IEnumerable<(string, MethodInfo)> PropertiesWithValidation(object instance)
{
var result = new List<string>();
foreach (PropertyInfo property in ReflectionHelper.GetPropertyInfos(instance))
foreach (PropertyInfo pInfo in ReflectionHelper.GetPropertyInfos(instance))
{
var errorString = ValidateMethod(instance, property);
if (!string.IsNullOrEmpty(errorString))
{
result.Add(errorString);
}
}
var vma = ReflectionHelper.GetAttribute<ValidateMethodAttribute>(pInfo);

if (vma is null) continue;

return result;
var mInfo = ReflectionHelper.GetMethodInfo<ErrorDescriptors>(instance, vma.MethodName);
yield return (pInfo.Name, mInfo);
}
}

public static string ValidateProperty(object instance, string propertyName)
public static ErrorDescriptors ValidateAllProperties(ViewModelBase viewModelBase,
List<(string propertyName, MethodInfo mInfo)> validationMethodCache)
{
var property = ReflectionHelper.GetPropertyInfo(instance, propertyName);
ErrorDescriptors result = null;

if (property != null)
foreach (var validationCache in validationMethodCache)
{
return ValidateMethod(instance, property);
var invokeRes = (ErrorDescriptors)validationCache.mInfo.Invoke(viewModelBase, null);

if (result is null) result = new ErrorDescriptors();

result.AddRange(invokeRes);
}

return string.Empty;
return result ?? ErrorDescriptors.Empty;
}

private static string ValidateMethod(object instance, PropertyInfo property)
public static ErrorDescriptors ValidateProperty(ViewModelBase viewModelBase,
string propertyName,
List<(string propertyName, MethodInfo mInfo)> validationMethodCache)
{
var vma = ReflectionHelper.GetAttribute<ValidateMethodAttribute>(property);
ErrorDescriptors result = null;

if (vma != null)
foreach (var validationCache in validationMethodCache)
{
return ReflectionHelper.InvokeMethod<string>(instance, vma.MethodName);
}
else
{
return string.Empty;
if (validationCache.propertyName != propertyName) continue;

var invokeRes = (ErrorDescriptors)validationCache.mInfo.Invoke(viewModelBase, null);

if (result is null) result = new ErrorDescriptors();

result.AddRange(invokeRes);
}

return result ?? ErrorDescriptors.Empty;
}
}
}
}
@@ -4,30 +4,43 @@
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Reflection;
using WalletWasabi.Gui.ViewModels.Validation;
using WalletWasabi.Helpers;

namespace WalletWasabi.Gui.ViewModels
{
public class ViewModelBase : ReactiveObject, INotifyDataErrorInfo
{
private List<(string, MethodInfo)> ValidationMethodCache;
public event EventHandler<DataErrorsChangedEventArgs> ErrorsChanged;

public bool HasErrors => Validator.ValidateAllProperties(this).Any();
public ViewModelBase()
{
var vmc = Validator.PropertiesWithValidation(this).ToList();

if (vmc.Count == 0) return;

ValidationMethodCache = vmc;
}

public bool HasErrors => Validator.ValidateAllProperties(this, ValidationMethodCache).HasErrors;

public IEnumerable GetErrors(string propertyName)
{
var errorString = Validator.ValidateProperty(this, propertyName);
if (!string.IsNullOrEmpty(errorString))
var error = Validator.ValidateProperty(this, propertyName, ValidationMethodCache);

This conversation was marked as resolved by jmacato

This comment has been minimized.

Copy link
@yahiheb

yahiheb Sep 9, 2019

Collaborator

Empty line can be removed.

Suggested change
if (error.HasErrors)
{
return new List<string> { errorString };
return error;
}

return null;
return ErrorDescriptors.Empty;
}

protected void NotifyErrorsChanged(string propertyName)
{
ErrorsChanged?.Invoke(this, new DataErrorsChangedEventArgs(propertyName));
}
}
}
}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.