[BUG] Property validations backed by ModelObservableValidationBase are not bound to view

[ins0mniaque]

Describe the bug

When using a complex validation rule specifying a property name, e.g.

this.ValidationRule(
                m => m.TextInput2,
                m => m.WhenAnyValue(m1 => m1.TextInput1, m1 => m1.TextInput2).Select(both => both.Item1 == both.Item2),
                (vm, isValid) => isValid ? string.Empty : "Both inputs should be the same");

and then

this.BindValidation(ViewModel, vm => vm.TextInput2, view => view.TextInput2Error.Text);

results in no errors in view.TextInput2Error.

This happens because ValidationContext extensions ResolveFor and ResolveForMultiple have not been updated to use the IPropertyValidationComponent interface.

This line:

ReactiveUI.Validation/src/ReactiveUI.Validation/Extensions/ValidationContextExtensions.cs

Line 41 in aa218ee

.OfType<BasePropertyValidation<TViewModel, TViewModelProperty>>()

skips over ModelObservableValidationBase objects.

Steps To Reproduce

Use example mentioned before.

Expected behavior

Validation should bind to view.

Screenshots

Environment

• OS: Windows / .NET Core 3.0
• Device: Windows
• Version: commit aa218ee
• Working Version: None

but the bug is present on all platforms.

Additional context

I have fixed this bug by refactoring ValidationContext extensions to use the IPropertyValidationComponent interface. I attached the refactored extensions which use the GetPropertyPath extension mentioned in issue #60.

NOTE: There is another bug lurking in the ValidationContext extensions. The overloads with multiple property expressions have typos which reuse TProperty1 instead of TProperty2 or TProperty3:

ReactiveUI.Validation/src/ReactiveUI.Validation/Extensions/ValidationContextExtensions.cs

Lines 70 to 71 in aa218ee

	Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
	Expression<Func<TViewModel, TProperty1>> viewModelProperty2)

ReactiveUI.Validation/src/ReactiveUI.Validation/Extensions/ValidationContextExtensions.cs

Lines 110 to 112 in aa218ee

	Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
	Expression<Func<TViewModel, TProperty1>> viewModelProperty2,
	Expression<Func<TViewModel, TProperty1>> viewModelProperty3)

ReactiveUI.Validation/src/ReactiveUI.Validation/Extensions/ValidationContextMultipleExtensions.cs

Lines 58 to 59 in aa218ee

	Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
	Expression<Func<TViewModel, TProperty1>> viewModelProperty2)

ReactiveUI.Validation/src/ReactiveUI.Validation/Extensions/ValidationContextMultipleExtensions.cs

Lines 86 to 88 in aa218ee

	Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
	Expression<Func<TViewModel, TProperty1>> viewModelProperty2,
	Expression<Func<TViewModel, TProperty1>> viewModelProperty3)

I know you might be in the process of refactoring property validation, but if you are ready, I'll make a pull request.

ValidationContextExtensions.cs:

// <copyright file="ReactiveUI.Validation/src/ReactiveUI.Validation/Extensions/ValidationContextExtensions.cs" company=".NET Foundation">
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
// </copyright>

using System;
using System.Linq;
using System.Linq.Expressions;
using ReactiveUI.Validation.Components.Abstractions;
using ReactiveUI.Validation.Contexts;
using ReactiveUI.Validation.Exceptions;

namespace ReactiveUI.Validation.Extensions
{
    /// <summary>
    /// Extensions methods for <see cref="ValidationContext"/>.
    /// </summary>
    public static class ValidationContextExtensions
    {
        /// <summary>
        /// Resolves the property valuation for a specified property.
        /// </summary>
        /// <typeparam name="TViewModel">ViewModel type.</typeparam>
        /// <typeparam name="TViewModelProperty">ViewModel property type.</typeparam>
        /// <param name="context">ValidationContext instance.</param>
        /// <param name="viewModelProperty">ViewModel property.</param>
        /// <param name="strict">Indicates if the ViewModel property to find is unique.</param>
        /// <returns>Returns a <see cref="IPropertyValidationComponent{TViewModel}"/> object if has a single validation property,
        /// otherwise will throw a <see cref="MultipleValidationNotSupportedException"/> exception.</returns>
        /// <exception cref="MultipleValidationNotSupportedException">
        /// Thrown if the ViewModel property has more than one validation associated.
        /// </exception>
        public static IPropertyValidationComponent<TViewModel> ResolveFor<TViewModel, TViewModelProperty>(
            this ValidationContext context,
            Expression<Func<TViewModel, TViewModelProperty>> viewModelProperty,
            bool strict = true)
        {
            var validations = context.Validations
                .OfType<IPropertyValidationComponent<TViewModel>>()
                .Where(v => v.ContainsProperty(viewModelProperty, strict))
                .ToList();

            if (validations.Count > 1)
            {
                throw new MultipleValidationNotSupportedException(viewModelProperty.Body.GetPropertyPath());
            }

            return validations[0];
        }

        /// <summary>
        /// Resolves the property valuation for two properties.
        /// </summary>
        /// <typeparam name="TViewModel">ViewModel type.</typeparam>
        /// <typeparam name="TProperty1">ViewModel first property type.</typeparam>
        /// <typeparam name="TProperty2">ViewModel second property type.</typeparam>
        /// <param name="context">ValidationContext instance.</param>
        /// <param name="viewModelProperty1">First ViewModel property.</param>
        /// <param name="viewModelProperty2">Second ViewModel property.</param>
        /// <returns>Returns a <see cref="IPropertyValidationComponent{TViewModel}"/> object if has a single validation property,
        /// otherwise will throw a <see cref="MultipleValidationNotSupportedException"/> exception.</returns>
        /// <exception cref="MultipleValidationNotSupportedException">
        /// Thrown if the ViewModel property has more than one validation associated.
        /// </exception>
        public static IPropertyValidationComponent<TViewModel> ResolveFor<TViewModel, TProperty1,
            TProperty2>(
            this ValidationContext context,
            Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
            Expression<Func<TViewModel, TProperty2>> viewModelProperty2)
        {
            var validations = context
                .Validations
                .OfType<IPropertyValidationComponent<TViewModel>>()
                .Where(v =>
                    v.ContainsProperty(viewModelProperty1) && v.ContainsProperty(viewModelProperty2)
                    && v.PropertyCount == 2)
                .ToList();

            if (validations.Count > 1)
            {
                throw new MultipleValidationNotSupportedException(
                    viewModelProperty1.Body.GetPropertyPath(),
                    viewModelProperty2.Body.GetPropertyPath());
            }

            return validations[0];
        }

        /// <summary>
        /// Resolves the property valuation for three properties.
        /// </summary>
        /// <typeparam name="TViewModel">ViewModel type.</typeparam>
        /// <typeparam name="TProperty1">ViewModel first property type.</typeparam>
        /// <typeparam name="TProperty2">ViewModel second property type.</typeparam>
        /// <typeparam name="TProperty3">ViewModel third property type.</typeparam>
        /// <param name="context">ValidationContext instance.</param>
        /// <param name="viewModelProperty1">First ViewModel property.</param>
        /// <param name="viewModelProperty2">Second ViewModel property.</param>
        /// <param name="viewModelProperty3">Third ViewModel property.</param>
        /// <returns>Returns a <see cref="IPropertyValidationComponent{TViewModel}"/> object if has a single validation property,
        /// otherwise will throw a <see cref="MultipleValidationNotSupportedException"/> exception.</returns>
        /// <exception cref="MultipleValidationNotSupportedException">
        /// Thrown if the ViewModel property has more than one validation associated.
        /// </exception>
        public static IPropertyValidationComponent<TViewModel> ResolveFor<TViewModel,
            TProperty1, TProperty2, TProperty3>(
            this ValidationContext context,
            Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
            Expression<Func<TViewModel, TProperty2>> viewModelProperty2,
            Expression<Func<TViewModel, TProperty3>> viewModelProperty3)
        {
            var validations = context
                .Validations
                .OfType<IPropertyValidationComponent<TViewModel>>()
                .Where(v =>
                    v.ContainsProperty(viewModelProperty1) && v.ContainsProperty(viewModelProperty2)
                                                           && v.ContainsProperty(viewModelProperty3)
                                                           && v.PropertyCount == 3)
                .ToList();

            if (validations.Count > 1)
            {
                throw new MultipleValidationNotSupportedException(
                    viewModelProperty1.Body.GetPropertyPath(),
                    viewModelProperty2.Body.GetPropertyPath(),
                    viewModelProperty3.Body.GetPropertyPath());
            }

            return validations[0];
        }
    }
}

ValidationContextMultipleExtensions.cs:

// <copyright file="ReactiveUI.Validation/src/ReactiveUI.Validation/Extensions/ValidationContextMultipleExtensions.cs" company=".NET Foundation">
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.
// </copyright>

using System;
using System.Collections.Generic;
using System.Linq;
using System.Linq.Expressions;
using ReactiveUI.Validation.Components.Abstractions;
using ReactiveUI.Validation.Contexts;

namespace ReactiveUI.Validation.Extensions
{
    /// <summary>
    /// Extensions methods for <see cref="ValidationContext"/> which supports multiple validations.
    /// </summary>
    public static class ValidationContextMultipleExtensions
    {
        /// <summary>
        /// Resolves all the properties valuations for a specified property.
        /// </summary>
        /// <typeparam name="TViewModel">ViewModel type.</typeparam>
        /// <typeparam name="TViewModelProperty">ViewModel property type.</typeparam>
        /// <param name="context">ValidationContext instance.</param>
        /// <param name="viewModelProperty">ViewModel property.</param>
        /// <param name="strict">Indicates if the ViewModel property to find is unique.</param>
        /// <returns>Returns a <see cref="IPropertyValidationComponent{TViewModel}"/> object.</returns>
        public static IEnumerable<IPropertyValidationComponent<TViewModel>> ResolveForMultiple<TViewModel,
            TViewModelProperty>(
            this ValidationContext context,
            Expression<Func<TViewModel, TViewModelProperty>> viewModelProperty,
            bool strict = true)
        {
            var validations = context.Validations
                .OfType<IPropertyValidationComponent<TViewModel>>()
                .Where(v => v.ContainsProperty(viewModelProperty, strict));

            return validations;
        }

        /// <summary>
        /// Resolves the property valuation for two properties.
        /// </summary>
        /// <typeparam name="TViewModel">ViewModel type.</typeparam>
        /// <typeparam name="TProperty1">ViewModel first property type.</typeparam>
        /// <typeparam name="TProperty2">ViewModel second property type.</typeparam>
        /// <param name="context">ValidationContext instance.</param>
        /// <param name="viewModelProperty1">First ViewModel property.</param>
        /// <param name="viewModelProperty2">Second ViewModel property.</param>
        /// <returns>Returns a <see cref="IPropertyValidationComponent{TViewModel}"/> object.</returns>
        public static IEnumerable<IPropertyValidationComponent<TViewModel>> ResolveForMultiple<
            TViewModel,
            TProperty1, TProperty2>(
            this ValidationContext context,
            Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
            Expression<Func<TViewModel, TProperty2>> viewModelProperty2)
        {
            var validations = context
                .Validations
                .OfType<IPropertyValidationComponent<TViewModel>>()
                .Where(v => v.ContainsProperty(viewModelProperty1) && v.ContainsProperty(viewModelProperty2)
                                                                   && v.PropertyCount == 2);

            return validations;
        }

        /// <summary>
        /// Resolves the property valuation for three properties.
        /// </summary>
        /// <typeparam name="TViewModel">ViewModel type.</typeparam>
        /// <typeparam name="TProperty1">ViewModel first property type.</typeparam>
        /// <typeparam name="TProperty2">ViewModel second property type.</typeparam>
        /// <typeparam name="TProperty3">ViewModel third property type.</typeparam>
        /// <param name="context">ValidationContext instance.</param>
        /// <param name="viewModelProperty1">First ViewModel property.</param>
        /// <param name="viewModelProperty2">Second ViewModel property.</param>
        /// <param name="viewModelProperty3">Third ViewModel property.</param>
        /// <returns>Returns a <see cref="IPropertyValidationComponent{TViewModel}"/> object.</returns>
        public static IEnumerable<IPropertyValidationComponent<TViewModel>>
            ResolveForMultiple<
                TViewModel, TProperty1, TProperty2, TProperty3>(
                this ValidationContext context,
                Expression<Func<TViewModel, TProperty1>> viewModelProperty1,
                Expression<Func<TViewModel, TProperty2>> viewModelProperty2,
                Expression<Func<TViewModel, TProperty3>> viewModelProperty3)
        {
            var validations = context
                .Validations
                .OfType<IPropertyValidationComponent<TViewModel>>()
                .Where(v => v.ContainsProperty(viewModelProperty1) && v.ContainsProperty(viewModelProperty2)
                                                                   && v.ContainsProperty(viewModelProperty3)
                                                                   && v.PropertyCount == 3);

            return validations;
        }
    }
}

[open-collective-bot]

Hey @ins0mniaque 👋,

Thank you for opening an issue. We will get back to you as soon as we can. Also, check out our Open Collective and consider contributing financially.

https://opencollective.com/reactiveui

PS.: We offer priority support for all financial contributors. Don't forget to add priority label once you start contributing 😄

ReactiveUI - Open Collective

An advanced, composable, functional reactive model-view-viewmodel framework for all .NET platforms!

[worldbeater]

Thanks for such a detailed report!

I know you might be in the process of refactoring property validation, but if you are ready, I'll make a pull request.

A pull request with the fixes would be highly appreciated. We might not be completely ready with the process of refactoring the library, but anyway we'll be happy to accept the fixes. Thank you! ❤️

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.