C# InputValidation mit INotifyDataErrorInfo + Async-Methode

[Thaxll'ssillyia]

Hallo Community!

Ich sitze seit ein paar Stunden an einem Problem, welches in meiner C#-WPF Anwendung auftritt. Zur Vereinfachung des Problems hab ich das mal auf nen Beispiel reduziert.

Ausgangslage:

Ich hab ein ViewModel mit INotifyDataErrorInfo-Implementierung. Zwei String-Properties (Firstname und Lastname) sind mit Validator-Attributen ausgestattet (MaxLength und Required). Ich nutze für die Validierung einen (teilweise) selbst geschriebenen ErrorValidationHelper, der auch einwandfrei funktioniert (mehrfach durch UnitTests abgesichert).
Im ViewModel wird beim Konstruktor zwei Werte für die strings in Temp-Felder gelegt, die dann in der (asynchronen) LoadData-Methode den Properties übergeben werden. Außerdem wird in der Load-Data Methode ein Task gestartet (im Original eine WCF-Methode awaited).

Problembeschreibung:

Scheinbar völlig undeterministisch sind die von WPF standardmäßig gezeichneten roten Balken um das Textfeld beim erstmaligen Aufruf der View da oder nicht da. Richtigerweise müssten diese nicht da sein, da die Bedingungen der Validators erfüllt sind (nicht leer und nicht zu lang). Wenn sie rot markiert sind, kann ich die Textboxen leer machen und neu befüllen, dann klappt die Validierung auf der Oberfläche.

Jetzt kommts: Lasse ich den Task mit dem Sleep(0) weg, funktioniert alles bestens. Ebenfalls funktioniert es, wenn ich das Window nicht als Child öffne, sondern als Hauptfenster. Erhöhe ich den Sleep-Wert auf bsp. 20, ist die Validierung nahezu immer fehlerhaft.

Es handelt sich um ein Problem der Oberfläche. Ich vermute hier konkurrierende Tasks, wobei ich den Fehler erstmal nicht blicke. Kann mir jemand helfen? Danke schonmal!

Im Anhang auch nochmal das VS-Projekt zum Testen (enthält auch bereits kompilierte exe).

ViewModel des Childs

namespace InputValidationTesting.ViewModel {
    using System;
    using System.Collections;
    using System.ComponentModel;
    using System.Runtime.CompilerServices;
    using System.Threading;
    using System.Threading.Tasks;

    using InputValidationTesting.Helper;

    public class CustomerEditViewModel : INotifyPropertyChanged, INotifyDataErrorInfo {

        private readonly ErrorValidationHelper<CustomerEditViewModel> _errorValidationHelper;
        private readonly string _firstnameTemp;
        private readonly string _lastnameTemp;

        private string _firstname;
        private string _lastname;

        public CustomerEditViewModel() {
            _firstnameTemp = "Vorname1";
            _lastnameTemp = "Vorname2";
            _errorValidationHelper = new ErrorValidationHelper<CustomerEditViewModel>(this);
        }

        public event EventHandler<DataErrorsChangedEventArgs> ErrorsChanged;

        public event PropertyChangedEventHandler PropertyChanged;

        /// <summary>
        ///     Liefert oder setzt den Vornamen des Kunden.
        /// </summary>
        [Required(AllowEmptyStrings = false, ErrorMessage = "Vorname wird benötigt!")]
        [MaxLength(256, ErrorMessage = "Maximal 256 Zeichen!")]
        public string Firstname {
            get { return _firstname; }
            set {
                _firstname = value;
                OnPropertyChanged();
            }
        }

        public bool HasErrors {
            get { return _errorValidationHelper.HasErrors; }
        }

        /// <summary>
        ///     Liefert oder setzt den Nachnamen des Kunden.
        /// </summary>
        [Required(AllowEmptyStrings = false, ErrorMessage = "Nachname wird benötigt!")]
        [MaxLength(256, ErrorMessage = "Maximal 256 Zeichen!")]
        public string Lastname {
            get { return _lastname; }
            set {
                _lastname = value;
                OnPropertyChanged();
            }
        }

        public IEnumerable GetErrors(string propertyName) {
            return _errorValidationHelper.GetErrors(propertyName);
        }

        public void InvokeErrorsChanged(DataErrorsChangedEventArgs e) {
            ErrorsChanged?.Invoke(this, e);
        }

        public async void LoadData() {
            await Task.Run(() => {
                               Thread.Sleep(0);
                           });

            Firstname = _firstnameTemp;
            Lastname = _lastnameTemp;
        }

        protected void OnPropertyChanged([CallerMemberName] string propertyName = null) {
            PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(propertyName));
        }

    }
}

ErrorValidationHelper

namespace InputValidationTesting.Helper {
    using System;
    using System.Collections;
    using System.Collections.Generic;
    using System.ComponentModel;
    using System.Linq;
    using System.Linq.Expressions;
    using System.Reflection;

    using InputValidationTesting.ViewModel;

    public class ErrorValidationHelper<T>
            where T : CustomerEditViewModel {

        private readonly IDictionary<String, IList<String>> _errors = new Dictionary<string, IList<string>>();

        private readonly IDictionary<string, KeyValuePair<Func<T, object>, ValidationBaseAttribute[]>> _validators;

        private readonly T _viewModel;

        public ErrorValidationHelper(T viewModel) {
            if (viewModel == null) {
                throw new ArgumentNullException(nameof(viewModel));
            }

            _viewModel = viewModel;
            _validators = new Dictionary<string, KeyValuePair<Func<T, object>, ValidationBaseAttribute[]>>();
            foreach (PropertyInfo property in typeof(T).GetProperties()) {
                ValidationBaseAttribute[] validations = GetValidations(property);
                if (validations.Length > 0) {
                    _validators.Add(property.Name,
                                    new KeyValuePair<Func<T, object>, ValidationBaseAttribute[]>(CreateValueGetter(property), validations));
                    Validate(_viewModel, property.Name);
                }
            }

            viewModel.PropertyChanged += delegate(object sender, PropertyChangedEventArgs args) {
                                             Validate(viewModel, args.PropertyName);
                                         };
        }

        public bool HasErrors {
            get { return _errors.Count > 0; }
        }

        public void AddError(string propertyName, string error, bool isWarning) {
            if (!_errors.ContainsKey(propertyName))
                _errors[propertyName] = new List<string>();

            if (!_errors[propertyName].Contains(error)) {
                if (isWarning)
                    _errors[propertyName].Add(error);
                else
                    _errors[propertyName].Insert(0, error);
                RaiseErrorsChanged(propertyName);
            }
        }

        public IEnumerable GetErrors(string propertyName) {
            if (string.IsNullOrEmpty(propertyName) || !_errors.ContainsKey(propertyName))
                return null;
            return _errors[propertyName];
        }

        public void RaiseErrorsChanged(string propertyName) {
            _viewModel.InvokeErrorsChanged(new DataErrorsChangedEventArgs(propertyName));
        }

        public void RemoveError(string propertyName, string error) {
            if (_errors.ContainsKey(propertyName) && _errors[propertyName].Contains(error)) {
                _errors[propertyName].Remove(error);
                if (_errors[propertyName].Count == 0)
                    _errors.Remove(propertyName);
                RaiseErrorsChanged(propertyName);
            }
        }

        public void Validate(T source, string columnName) {
            KeyValuePair<Func<T, object>, ValidationBaseAttribute[]> validators;
            bool isWatched = _validators.TryGetValue(columnName, out validators);
            if (isWatched) {
                object value = validators.Key(source);
                ValidationBaseAttribute[] validationAttributes = validators.Value;

                IEnumerable<ValidationBaseAttribute> invalidAttributes = validationAttributes.Where(v => !v.IsValid(value));
                IEnumerable<ValidationBaseAttribute> validAttributes = validationAttributes.Where(v => v.IsValid(value));

                foreach (ValidationBaseAttribute validationAttribute in invalidAttributes) {
                    AddError(columnName, validationAttribute.GetErrorMessageString(), false);
                }

                foreach (ValidationBaseAttribute validationAttribute in validAttributes) {
                    RemoveError(columnName, validationAttribute.GetErrorMessageString());
                }
            }
        }

        private static Func<T, object> CreateValueGetter(PropertyInfo property) {
            ParameterExpression instance = Expression.Parameter(typeof(T), "i");
            UnaryExpression cast = Expression.TypeAs(Expression.Property(instance, property), typeof(object));
            return (Func<T, object>)Expression.Lambda(cast, instance).Compile();
        }

        private static ValidationBaseAttribute[] GetValidations(PropertyInfo property) {
            return (ValidationBaseAttribute[])property.GetCustomAttributes(typeof(ValidationBaseAttribute), true);
        }

    }
}

Anmerkung:

Der ErrorValidationHelper arbeitet nach folgendem Prinzip:

- Bei dessen Erzeugung werden alle Properties mit Validator-Attribut des generic types identifiziert, dann werden Accessoren für die entsprechenden Properties erzeugt und anschließend alle Properties validiert
- Die Validierung funktioniert über Error-Listen, wo die Fehler hinzugefügt oder gelöscht werden
- Bei jedem PropertyChange wird die Validierung für das entsprechende Property neu durchgeführt

 

[cremor]

Ich habe deine Solution jetzt zwar nicht in VS aufgemacht, aber eine Idee:

Du awaitest die LoadData Methode nicht, wodurch das Setzen des DataContexts und das Öffnen des Dialogs vor oder nach dem Beenden dessen Task - und damit dem Setzen der Properties im ViewModel - passieren kann. Je nachdem welcher Thread gerade zuerst dran ist, kann sich die Reihenfolge ändern, das würde die unterschiedlichen Ergebnisse erklären. Warum gibt LoadData nicht einen Task zurück, der awaited werden kann?
Generelle Best Practice: Niemals async void Methoden machen, außer es ist ein Event Handler. Und alle async Methoden immer awaiten, außer man will explizit, dass etwas parallel passiert.

Hat zwar nichts mit der Ausführung zu tun, aber viele sehen es auch als Best Practice an, wenn async Methoden (genauer gesagt Methoden, die Tasks zurückgeben) ein "Async" Suffix im Namen haben.

 

[Thaxll'ssillyia]

Danke, du hast mein Problem gelöst!

Vermutlich hab ich noch nen Verständnisproblem mit async-await...ich kam bisher halt von der Threads. Ich werd mir die Sache nochmal genauer anschauen müssen für mich zum Verständnis. Danke auch für den Codestyle-Hinweis!

Anmerkung:

Da hätte ich aber doch noch die Frage, wie async await bei Properties angewendet wird.
Ich hab ein IsSelected-Property, welches wenn es true gesetzt wird, die async Task LoadData-Methode aufrufen soll. Das Property kann ich aber logischerweise nicht async machen, ist dann die Lösung die Methode als async void zu kennzeichnen? (siehe hier: https://stackoverflow.com/questions...ce=google_rich_qa&utm_campaign=google_rich_qa Antwort 9)

 

[Bagbag]

Eine Property kann nicht asynchron sein, das beste was du hier machen kannst ist den Task der aufgerufenen Funktion zurückgeben, auf den du dann awaiten darfst.

Da du beim setter eine Load-Methode aufrufen willst, vermute ich aber, dass du hier etwas unschön löst (wenn ich irgendwo einen Wert setzte, gehe ich definitiv nicht davon aus, dass etwas geladen wird).

Magst du den Code mal posten?

 

[Thaxll'ssillyia]

Die Module sind hierarchisch angeordnet. Wenn ein Modul selektiert wird, soll es seine Daten neu laden.

/// <summary>
        ///     Liefert oder setzt die Angabe, ob das Modul selektiert ist.
        /// </summary>
        public bool IsSelected
        {
            get { return _isSelected; }
            set
            {
                if (value && !IsSelectable) {
                    _logger.Error($"Module {GetType()} is about to be selected, but isn't allowed of it's permissions!");
                    return;
                }

                // Ist schon ausgewählt -> Nix tun
                if (_isSelected == value) {
                    return;
                }

                _isSelected = value;
                OnPropertyChanged(this.GetPropertyName(m => m.IsSelected));

                PerformSelection();
            }
        }

        private async void PerformSelection() {

            // Events
            InvokeSelected();
            OnModuleSelected();

            if (_isSelected) {

                // Parent-Module selektieren
                SelectParent();

                if (Parent != null) {
                    // Anderen Module deselektieren
                    foreach (IModuleBase moduleBase in ((ParentModuleBase)Parent).SubModules.Except(new[] { this })) {
                        moduleBase.IsSelected = false;
                    }
                }

                if (_loadDataOnSelection) {
                    await LoadData();
                }
            }
        }

 

[Zonk91]

Du könntest es natürlich so machen. Hat natürlich den Nachteil, dass du die Vorteile von async/await etc. verliertst. Generell würde ich ein Refactoring empfehlen. In gettern/settern Logik einzubauen ist finde ich zumindest, nicht die feine Englische.

  public bool IsSelected
        {
            get { return true; }
            set { PerformSelection().RunSynchronously(); }
        }

        public async Task PerformSelection()
        {
            // Events
            InvokeSelected();
            OnModuleSelected();
            if (_isSelected)
            {
                // Parent-Module selektieren
                SelectParent();
                if (Parent != null)
                {
                    // Anderen Module deselektieren
                    foreach (IModuleBase moduleBase in ((ParentModuleBase) Parent).SubModules.Except(new[] {this}))
                    {
                        moduleBase.IsSelected = false;
                    }
                }

                if (_loadDataOnSelection)
                {
                    await LoadData();
                }
            }
        }

 

[Bagbag]

Meines erachtens nach missbrauchst du hier Properties. Ich würde hier statt "IsSelected = true;" einfach ein "await Select();" aufrufen.

Falls ich noch weiteren Senf abgeben darf: Ohne genau zu wissen was du da machst: Ein SubModule sollte nicht für das Deselektieren der Siblings zuständig sein.

 

[Thaxll'ssillyia]

Das Problem ist, dass die Module auf der Oberfläche Tabs in einem Tabcontrol sind, und damit das IsSelected auch von der Oberfläche aus stattfindet. Das Deselektieren muss ich machen, wenn ich das Modul manuell selektiere (übers Tab erledigt das sonst das TabControl). Ich wüsste nicht wo ich sonst die Logik für das Deselktieren der Siblings hin tun soll.

Ich würde halt trotzdem wollen, dass bei Tabwechsel das Laden der Daten asynchron erfolgt, also schon während des Wechsel in der Oberfläche.

 

[Bagbag]

Beim Anbinden kann ich dir jetzt nicht konrekt weiter helfen, dafür fehlt mir die Kenntnis der API. Aber prinzipiell muss irgendwo ein Schnitt zwischen synchron und asynchron erfolgen, daher ist es gut möglich, dass bei deiner jetzigen Umsetzung mit diesem Framework es nicht besser lösbar ist.

Die Trennung der Logik und Funktionalität hätte ich so gemacht (vorausgesetzt, ich habe deine Intentionen an dem Code den du gepostet hast richtig interpretiert...):

Spoiler

public partial class MainWindow : Window
    {
        private Router router;

        public bool IsFirstStart { get; set; }

        public MainWindow()
        {
            router = new Router();

            InitializeComponent();
        }

        private void Initialize()
        {
            ConfigureRoutes();

            if (IsFirstStart)
            {
                router.TryNavigateToRoute(Routes.Welcome);
            }
        }

        private void ConfigureRoutes()
        {
            router.ConfigureRoute(Routes.Main, () => new RouteControl(router));
            router.ConfigureRoute(Routes.Welcome, () => new RouteControl(router));
            router.ConfigureRoute(Routes.Settings, () => new RouteControl(router));
            router.ConfigureRoute(Routes.News, () => new RouteControl(router));
        }
    }

    public static class Routes
    {
        public static string Main => nameof(Main);
        public static string Welcome => nameof(Welcome);
        public static string News => nameof(News);
        public static string Settings => nameof(Settings);
    }

    public class Router
    {
        public delegate RouteControl RouteControlBuilder();

        private RouteManager routeManager;
        private IDictionary<string, RouteControlBuilder> routeControlBuilders;
        private IDictionary<string, RouteControl> routeControls;

        public Router()
        {
            routeManager = new RouteManager();
        }

        public void ConfigureRoute(string route, RouteControlBuilder routeControlBuilder)
        {
            routeControlBuilders[route] = routeControlBuilder;
        }

        public async Task<bool> TryNavigateToRoute(string route)
        {
            var routeControl = null as RouteControl;
            var hasRouteControl = routeControls.TryGetValue(route, out routeControl);

            if (!hasRouteControl)
            {
                routeControl = BuildRouteControl(route);
                routeControls[route] = routeControl;
            }

            var success = await routeManager.TryActivateRoute(routeControl);
            return success;
        }

        private RouteControl BuildRouteControl(string route)
        {
            var builder = routeControlBuilders[route];
            var routeControl = builder();

            routeControl.OnRequestActivation += (sender, args) => TryNavigateToRoute(route);

            return routeControl;
        }
    }

    public class RouteManager
    {
        private RouteControl activeRoute;

        public async Task<bool> TryActivateRoute(RouteControl routeControl)
        {
            var success =
                await routeControl.CanActivate() &&
                await DeactivateCurrentRoute();

            if (success)
            {
                await routeControl.OnActivate();
            }

            return success;
        }

        private async Task<bool> DeactivateCurrentRoute()
        {
            var canLeaveCurrentRoute = true;
            if (activeRoute != null)
            {
                canLeaveCurrentRoute = await activeRoute.OnDeactivate();
            }

            return canLeaveCurrentRoute;
        }
    }

    public class RouteControl
    {
        private Router router;
        private bool isSelected = false;

        public event EventHandler OnRequestActivation;

        public bool IsSelected
        {
            get { return isSelected; }
            set
            {
                if (value == isSelected)
                {
                    return;
                }

                isSelected = value;

                if (isSelected)
                {
                    RequestSelection();
                }
            }
        }

        public RouteControl(Router router)
        {
            this.router = router;

            LetSomethingImportantHappenSoon();
        }

        public async Task<bool> OnDeactivate()
        {
            // warn about unsaved data or whatever
            await Task.Delay(100);

            // block leaving by returning false
            return true;
        }

        public async Task<bool> CanActivate()
        {
            // check for authorization
            await Task.Delay(100);

            // block selection by returning false
            return true;
        }

        public async Task OnActivate()
        {
            // load data
            await Task.Delay(100);
        }

        private void LetSomethingImportantHappenSoon()
        {
            Task.Delay(5000).ContinueWith((previousTask) => RequestSelection());
        }

        private void RequestSelection()
        {
            if (OnRequestActivation != null)
            {
                OnRequestActivation(this, null);
            }
        }
    }

RouteControl muss natürlich ein Interface oder eine (abstrakte) Basisklasse sein und dann pro View implementiert werden.