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

Fixes ReactiveValidationObject NullReferenceException and enhances performance and thread-safety of ValidationText #144

Merged
merged 12 commits into from
Oct 20, 2020
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,29 @@ namespace ReactiveUI.Validation.Collections
{
public class ValidationText : System.Collections.Generic.IEnumerable<string>, System.Collections.IEnumerable
{
public static readonly ReactiveUI.Validation.Collections.ValidationText Empty;
[System.Obsolete("Calling the constructor is deprecated, please use ValidationText.Create() overloa" +
"d instead.")]
public ValidationText() { }
[System.Obsolete("Calling the constructor is deprecated, please use ValidationText.Create(IEnumerab" +
"le<ValidationText>) overload instead.")]
public ValidationText(System.Collections.Generic.IEnumerable<ReactiveUI.Validation.Collections.ValidationText> validationTexts) { }
[System.Obsolete("Calling the constructor is deprecated, please use ValidationText.Create(string) o" +
"verload instead.")]
public ValidationText(string text) { }
public int Count { get; }
public string this[int index] { get; }
[System.Obsolete("ValidationText will be made immutable in future versions, please do not use the A" +
"dd(string) method.")]
public void Add(string text) { }
[System.Obsolete("ValidationText will be made immutable in future versions, please do not use the C" +
"lear() method.")]
public void Clear() { }
public System.Collections.Generic.IEnumerator<string> GetEnumerator() { }
public string ToSingleLine(string? separator = ",") { }
public static ReactiveUI.Validation.Collections.ValidationText Create(System.Collections.Generic.IEnumerable<ReactiveUI.Validation.Collections.ValidationText> validationTexts) { }
public static ReactiveUI.Validation.Collections.ValidationText Create(System.Collections.Generic.IEnumerable<string> validationTexts) { }
public static ReactiveUI.Validation.Collections.ValidationText Create(params string[] validationTexts) { }
}
}
namespace ReactiveUI.Validation.Comparators
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,29 @@ namespace ReactiveUI.Validation.Collections
{
public class ValidationText : System.Collections.Generic.IEnumerable<string>, System.Collections.IEnumerable
{
public static readonly ReactiveUI.Validation.Collections.ValidationText Empty;
[System.Obsolete("Calling the constructor is deprecated, please use ValidationText.Create() overloa" +
"d instead.")]
public ValidationText() { }
[System.Obsolete("Calling the constructor is deprecated, please use ValidationText.Create(IEnumerab" +
"le<ValidationText>) overload instead.")]
public ValidationText(System.Collections.Generic.IEnumerable<ReactiveUI.Validation.Collections.ValidationText> validationTexts) { }
[System.Obsolete("Calling the constructor is deprecated, please use ValidationText.Create(string) o" +
"verload instead.")]
public ValidationText(string text) { }
public int Count { get; }
public string this[int index] { get; }
[System.Obsolete("ValidationText will be made immutable in future versions, please do not use the A" +
"dd(string) method.")]
public void Add(string text) { }
[System.Obsolete("ValidationText will be made immutable in future versions, please do not use the C" +
"lear() method.")]
public void Clear() { }
public System.Collections.Generic.IEnumerator<string> GetEnumerator() { }
public string ToSingleLine(string? separator = ",") { }
public static ReactiveUI.Validation.Collections.ValidationText Create(System.Collections.Generic.IEnumerable<ReactiveUI.Validation.Collections.ValidationText> validationTexts) { }
public static ReactiveUI.Validation.Collections.ValidationText Create(System.Collections.Generic.IEnumerable<string> validationTexts) { }
public static ReactiveUI.Validation.Collections.ValidationText Create(params string[] validationTexts) { }
}
}
namespace ReactiveUI.Validation.Comparators
Expand Down
4 changes: 2 additions & 2 deletions src/ReactiveUI.Validation.Tests/ValidationBindingTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -706,7 +706,7 @@ private class CustomValidationState : IValidationState
public CustomValidationState(bool isValid, string message)
{
IsValid = isValid;
Text = new ValidationText(isValid ? string.Empty : message);
Text = isValid ? ValidationText.Empty : ValidationText.Create(message);
}

public ValidationText Text { get; }
Expand All @@ -723,4 +723,4 @@ private class ConstFormatter : IValidationTextFormatter<string>
public string Format(ValidationText validationText) => _text;
}
}
}
}
73 changes: 60 additions & 13 deletions src/ReactiveUI.Validation/Collections/ValidationText.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,10 @@
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for full license information.

using System;
using System.Collections;
using System.Collections.Generic;
using System.Linq;

namespace ReactiveUI.Validation.Collections
{
Expand All @@ -13,79 +15,124 @@ namespace ReactiveUI.Validation.Collections
/// </summary>
public class ValidationText : IEnumerable<string>
{
private readonly List<string> _texts = new List<string>();
/// <summary>
/// The empty validation text singleton instance, contains a single empty string.
/// </summary>
public static readonly ValidationText Empty = new ValidationText(Array.Empty<string>());

// TODO When Add() & Clear() are obsolesced this should be made read-only.
glennawatson marked this conversation as resolved.
Show resolved Hide resolved
private /* readonly */ string[] _texts;

/// <summary>
/// Initializes a new instance of the <see cref="ValidationText"/> class.
/// </summary>
[Obsolete("Calling the constructor is deprecated, please use ValidationText.Create() overload instead.")]
worldbeater marked this conversation as resolved.
Show resolved Hide resolved
public ValidationText()
: this(Array.Empty<string>())
{
}

/// <summary>
/// Initializes a new instance of the <see cref="ValidationText"/> class.
/// </summary>
/// <param name="text">Text to be added in the collection.</param>
[Obsolete("Calling the constructor is deprecated, please use ValidationText.Create(string) overload instead.")]
public ValidationText(string text)
: this(new[] { text })
{
_texts.Add(text);
}

/// <summary>
/// Initializes a new instance of the <see cref="ValidationText"/> class.
/// </summary>
/// <param name="validationTexts"><see cref="ValidationText"/> collection to be added into the text collection.</param>
[Obsolete("Calling the constructor is deprecated, please use ValidationText.Create(IEnumerable<ValidationText>) overload instead.")]
public ValidationText(IEnumerable<ValidationText> validationTexts)
: this((validationTexts ?? throw new ArgumentNullException(nameof(validationTexts))).SelectMany(vt => vt._texts).ToArray())
{
if (validationTexts is null)
{
throw new System.ArgumentNullException(nameof(validationTexts));
}
}

foreach (var text in validationTexts)
/// <summary>
/// Initializes a new instance of the <see cref="ValidationText"/> class with the array of texts.
/// </summary>
/// <param name="texts">The array of texts.</param>
private ValidationText(string[] texts)
{
if (texts is null)
{
_texts.AddRange(text._texts);
throw new ArgumentNullException(nameof(texts));
}

// TODO Can remove this check when public constructors are obsolesced as Create method already checks this.
_texts = texts.Length < 1
? Array.Empty<string>()
: texts;
}

/// <summary>
/// Gets returns the number of elements in the collection.
/// </summary>
public int Count => _texts.Count;
public int Count => _texts.Length;

/// <summary>
/// Indexer.
/// </summary>
/// <param name="index">Position.</param>
public string this[int index] => _texts[index];

/// <summary>
/// Combines multiple <see cref="ValidationText"/> instances into a single instance, or returns <see cref="Empty"/> if the enumeration is empty, or contains a single empty element.
/// </summary>
/// <param name="validationTexts">An enumeration of <see cref="ValidationText"/>.</param>
/// <returns>A <see cref="ValidationText"/>.</returns>
public static ValidationText Create(IEnumerable<ValidationText> validationTexts) =>
Create(validationTexts.SelectMany(vt => vt._texts).ToArray());

/// <summary>
/// Combines multiple validation messages into a single instance, or returns <see cref="Empty"/> if the enumeration is empty, or contains a single empty element.
/// </summary>
/// <param name="validationTexts">An enumeration of validation messages.</param>
/// <returns>A <see cref="ValidationText"/>.</returns>
public static ValidationText Create(IEnumerable<string> validationTexts) => Create(validationTexts.ToArray());

/// <summary>
/// Combines multiple validation messages into a single instance, or returns <see cref="Empty"/> if the enumeration is empty, or contains a single empty element.
/// </summary>
/// <param name="validationTexts">An array of validation messages.</param>
/// <returns>A <see cref="ValidationText"/>.</returns>
public static ValidationText Create(params string[] validationTexts) => validationTexts.Length < 1
? Empty
: new ValidationText(validationTexts);

/// <inheritdoc/>
public IEnumerator<string> GetEnumerator()
{
return _texts.GetEnumerator();
return ((IEnumerable<string>)_texts).GetEnumerator();
}

/// <inheritdoc/>
IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
return _texts.GetEnumerator();
}

/// <summary>
/// Adds a text to the collection.
/// </summary>
/// <param name="text">Text to be added in the collection.</param>
[Obsolete("ValidationText will be made immutable in future versions, please do not use the Add(string) method.")]
public void Add(string text)
{
_texts.Add(text);
_texts = _texts.Concat(new[] { text }).ToArray();
}

/// <summary>
/// Clear all texts.
/// </summary>
[Obsolete("ValidationText will be made immutable in future versions, please do not use the Clear() method.")]
public void Clear()
{
_texts.Clear();
_texts = Array.Empty<string>();
}

/// <summary>
Expand Down
11 changes: 4 additions & 7 deletions src/ReactiveUI.Validation/Components/BasePropertyValidation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -204,9 +204,7 @@ public BasePropertyValidation(
Expression<Func<TViewModel, TViewModelProperty>> viewModelProperty,
Func<TViewModelProperty, bool> isValidFunc,
string message)
: this(viewModel, viewModelProperty, isValidFunc, (p, v) => new ValidationText(v
? string.Empty
: message))
: this(viewModel, viewModelProperty, isValidFunc, (p, v) => v ? ValidationText.Empty : ValidationText.Create(message))
{
}

Expand All @@ -222,9 +220,8 @@ public BasePropertyValidation(
Expression<Func<TViewModel, TViewModelProperty>> viewModelProperty,
Func<TViewModelProperty, bool> isValidFunc,
Func<TViewModelProperty, string> message)
: this(viewModel, viewModelProperty, isValidFunc, (p, v) => new ValidationText(v
? string.Empty
: message(p)))
: this(viewModel, viewModelProperty, isValidFunc, (p, v) =>
v ? ValidationText.Empty : ValidationText.Create(message(p)))
{
}

Expand All @@ -241,7 +238,7 @@ public BasePropertyValidation(
Func<TViewModelProperty, bool> isValidFunc,
Func<TViewModelProperty, bool, string> messageFunc)
: this(viewModel, viewModelProperty, isValidFunc, (prop1, isValid) =>
new ValidationText(messageFunc(prop1, isValid)))
ValidationText.Create(messageFunc(prop1, isValid)))
{
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ public ModelObservableValidation(
Func<TViewModel, IObservable<bool>> validityObservable,
string message)
: this(viewModel, viewModelProperty, validityObservable, (p, isValid) =>
new ValidationText(isValid ? string.Empty : message))
isValid ? ValidationText.Empty : ValidationText.Create(message))
{
}

Expand All @@ -64,7 +64,7 @@ public ModelObservableValidation(
Func<TViewModel, IObservable<bool>> validityObservable,
Func<TViewModel, string> message)
: this(viewModel, viewModelProperty, validityObservable, (p, isValid) =>
new ValidationText(isValid ? string.Empty : message(p)))
isValid ? ValidationText.Empty : ValidationText.Create(message(p)))
{
}

Expand All @@ -81,7 +81,7 @@ public ModelObservableValidation(
Func<TViewModel, IObservable<bool>> validityObservable,
Func<TViewModel, bool, string> messageFunc)
: this(viewModel, viewModelProperty, validityObservable, (vm, state) =>
new ValidationText(messageFunc(vm, state)))
ValidationText.Create(messageFunc(vm, state)))
{
}

Expand Down Expand Up @@ -129,7 +129,7 @@ public ModelObservableValidation(
Func<TViewModel, IObservable<bool>> validityObservable,
string message)
: this(viewModel, validityObservable, (p, isValid) =>
new ValidationText(isValid ? string.Empty : message))
isValid ? ValidationText.Empty : ValidationText.Create(message))
{
}

Expand All @@ -144,7 +144,7 @@ public ModelObservableValidation(
Func<TViewModel, IObservable<bool>> validityObservable,
Func<TViewModel, string> message)
: this(viewModel, validityObservable, (p, isValid) =>
new ValidationText(isValid ? string.Empty : message(p)))
isValid ? ValidationText.Empty : ValidationText.Create(message(p)))
{
}

Expand All @@ -158,7 +158,7 @@ public ModelObservableValidation(
TViewModel viewModel,
Func<TViewModel, IObservable<bool>> validityObservable,
Func<TViewModel, bool, string> messageFunc)
: this(viewModel, validityObservable, (vm, state) => new ValidationText(messageFunc(vm, state)))
: this(viewModel, validityObservable, (vm, state) => ValidationText.Create(messageFunc(vm, state)))
{
}

Expand Down
4 changes: 2 additions & 2 deletions src/ReactiveUI.Validation/Components/ObservableValidation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public ObservableValidation(
Func<TViewModel, TValue, bool> isValidFunc,
Func<TViewModel, TValue, bool, string> messageFunc)
: base(viewModel, observable, isValidFunc, (vm, value, isValid) =>
new ValidationText(messageFunc(vm, value, isValid))) =>
ValidationText.Create(messageFunc(vm, value, isValid))) =>
AddProperty(viewModelProperty);

/// <summary>
Expand Down Expand Up @@ -217,7 +217,7 @@ public ObservableValidation(
Func<TViewModel, TValue, bool> isValidFunc,
Func<TViewModel, TValue, bool, string> messageFunc)
: base(viewModel, observable, isValidFunc, (vm, value, isValid) =>
new ValidationText(messageFunc(vm, value, isValid)))
ValidationText.Create(messageFunc(vm, value, isValid)))
{
}

Expand Down
4 changes: 2 additions & 2 deletions src/ReactiveUI.Validation/Contexts/ValidationContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ public ValidationContext(IScheduler? scheduler = null)
_validationText = _validSubject
.StartWith(true)
.Select(_ => BuildText())
.ToProperty(this, m => m.Text, new ValidationText(), scheduler: scheduler)
.ToProperty(this, m => m.Text, ValidationText.Empty, scheduler: scheduler)
.DisposeWith(_disposables);

_validSubject
Expand Down Expand Up @@ -219,7 +219,7 @@ private void Activate()
/// Returns the <see cref="ValidationText"/> with all the error messages from the non valid components.
/// </returns>
private ValidationText BuildText() =>
new ValidationText(_validations
ValidationText.Create(_validations
.Where(p => !p.IsValid && p.Text != null)
.Select(p => p.Text!));
}
Expand Down
5 changes: 3 additions & 2 deletions src/ReactiveUI.Validation/Helpers/ReactiveValidationObject.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
using DynamicData;
using DynamicData.Binding;
using ReactiveUI.Validation.Abstractions;
using ReactiveUI.Validation.Collections;
using ReactiveUI.Validation.Components.Abstractions;
using ReactiveUI.Validation.Contexts;
using ReactiveUI.Validation.States;
Expand Down Expand Up @@ -90,11 +91,11 @@ public bool HasErrors
public virtual IEnumerable GetErrors(string propertyName) =>
string.IsNullOrEmpty(propertyName) ?
SelectInvalidPropertyValidations()
.SelectMany(validation => validation.Text)
.SelectMany(validation => validation.Text ?? ValidationText.Empty)
.ToArray() :
SelectInvalidPropertyValidations()
.Where(validation => validation.ContainsPropertyName(propertyName))
.SelectMany(validation => validation.Text)
.SelectMany(validation => validation.Text ?? ValidationText.Empty)
.ToArray();

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/ReactiveUI.Validation/ReactiveUI.Validation.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

<PropertyGroup>
<TargetFrameworks>MonoAndroid90;Xamarin.iOS10;Xamarin.Mac20;Xamarin.TVOS10;tizen40;netstandard2.0</TargetFrameworks>
<TargetFrameworks Condition=" '$(OS)' == 'Windows_NT' ">$(TargetFrameworks);net461;uap10.0.17763</TargetFrameworks>
<TargetFrameworks Condition=" '$(OS)' == 'Windows_NT' ">$(TargetFrameworks);net461;</TargetFrameworks>
<NoWarn>$(NoWarn);CS1591</NoWarn>
<Nullable>enable</Nullable>
<LangVersion>latest</LangVersion>
Expand Down
4 changes: 2 additions & 2 deletions src/ReactiveUI.Validation/States/ValidationState.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ public class ValidationState : IValidationState
/// <param name="isValid">Determines if the property is valid or not.</param>
/// <param name="text">Validation text.</param>
public ValidationState(bool isValid, string text)
: this(isValid, new ValidationText(text))
: this(isValid, ValidationText.Create(text))
{
}

Expand All @@ -50,7 +50,7 @@ public ValidationState(bool isValid, ValidationText text)
[ExcludeFromCodeCoverage]
[Obsolete("This constructor overload is going to be removed soon.")]
public ValidationState(bool isValid, string text, IValidationComponent component)
: this(isValid, new ValidationText(text), component)
: this(isValid, ValidationText.Create(text), component)
{
}

Expand Down
Loading