diff --git a/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/BindingContextTests.cs b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/BindingContextTests.cs new file mode 100644 index 00000000000..5f5468362a0 --- /dev/null +++ b/src/System.Windows.Forms.Legacy/System.Windows.Forms.Legacy.Tests/BindingContextTests.cs @@ -0,0 +1,100 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.ComponentModel; +using System.Windows.Forms; + +namespace System.Windows.Forms.Legacy.Tests; + +public class BindingContextTests +{ + [Fact] + public void BindingContext_UpdateBinding_RelatedManagerCurrentItemChangedDuringPullData_DoesNotReenter() + { + BindingContext context = []; + BindingList parentList = null!; + ReentrantChildDataSource child = new(() => parentList.ResetItem(0)); + ReentrantParentDataSource parent = new(child); + parentList = [parent]; + + ReentrantBindableComponent component = new() { BindingContext = context }; + Binding binding = component.DataBindings.Add( + nameof(ReentrantBindableComponent.Value), + parentList, + $"{nameof(ReentrantParentDataSource.Children)}.{nameof(ReentrantChildDataSource.Value)}"); + + CurrencyManager relatedManager = Assert.IsAssignableFrom(context[parentList, nameof(ReentrantParentDataSource.Children)]); + Assert.Same(relatedManager, binding.BindingManagerBase); + + Exception? dataError = null; + relatedManager.DataError += (sender, e) => dataError = e.Exception; + + component.Value = "updated"; + parentList.ResetItem(0); + + Assert.Null(dataError); + Assert.Equal(1, child.ValueSetCount); + Assert.Equal("updated", child.Value); + } + + private class ReentrantParentDataSource + { + public ReentrantParentDataSource(ReentrantChildDataSource child) + { + Children = [child]; + } + + public BindingList Children { get; } + } + + private class ReentrantChildDataSource + { + private readonly Action _valueSetCallback; + private string _value = "initial"; + + public ReentrantChildDataSource(Action valueSetCallback) + { + _valueSetCallback = valueSetCallback; + } + + public int ValueSetCount { get; private set; } + + public string Value + { + get => _value; + set + { + ValueSetCount++; + if (ValueSetCount > 1) + { + throw new InvalidOperationException("Related manager pulled data re-entrantly."); + } + + _value = value; + _valueSetCallback(); + } + } + } + + private class ReentrantBindableComponent : BindableComponent + { + private string? _value; + + public string? Value + { + get => _value; + set + { + if (_value == value) + { + return; + } + + _value = value; + ValueChanged?.Invoke(this, EventArgs.Empty); + } + } + + public event EventHandler? ValueChanged; + } +} \ No newline at end of file diff --git a/src/System.Windows.Forms/System/Windows/Forms/DataBinding/RelatedCurrencyManager.cs b/src/System.Windows.Forms/System/Windows/Forms/DataBinding/RelatedCurrencyManager.cs index f0e87dea1ce..83169b6bb22 100644 --- a/src/System.Windows.Forms/System/Windows/Forms/DataBinding/RelatedCurrencyManager.cs +++ b/src/System.Windows.Forms/System/Windows/Forms/DataBinding/RelatedCurrencyManager.cs @@ -14,6 +14,7 @@ internal class RelatedCurrencyManager : CurrencyManager { private BindingManagerBase _parentManager; private PropertyDescriptor _fieldInfo; + private bool _handlingCurrentItemChanged; private static List IgnoreItemChangedTable { get; } = []; internal RelatedCurrencyManager(BindingManagerBase parentManager, string dataField) @@ -142,69 +143,82 @@ private void ParentManager_CurrentItemChanged(object? sender, EventArgs e) return; } - int oldlistposition = listposition; - - // we only pull the data from the controls into the backEnd. we do not care about keeping the lastGoodKnownRow - // when we are about to change the entire list in this currencymanager. - try - { - PullData(); - } - catch (Exception ex) + if (_handlingCurrentItemChanged) { - OnDataError(ex); + return; } - if (_parentManager is CurrencyManager currencyManager) + _handlingCurrentItemChanged = true; + try { - if (currencyManager.Count > 0) + int oldlistposition = listposition; + + // we only pull the data from the controls into the backEnd. we do not care about keeping the lastGoodKnownRow + // when we are about to change the entire list in this currencymanager. + try { - // Parent list has a current row, so get the related list from the relevant property on that row. - SetDataSource(_fieldInfo.GetValue(currencyManager.Current)); - listposition = (Count > 0 ? 0 : -1); + PullData(); } - else + catch (Exception ex) { - // APPCOMPAT: bring back the Everett behavior where the currency manager adds an item and - // then it cancels the addition. - // - // really, really hocky. - // will throw if the list in the curManager is not IBindingList - // and this will fail if the IBindingList does not have list change notification. read on.... - // when a new item will get added to an empty parent table, - // the table will fire OnCurrentChanged and this method will get executed again - // allowing us to set the data source to an object with the right properties (so we can show - // metadata at design time). - // we then call CancelCurrentEdit to remove the dummy row, but making sure to ignore any - // OnCurrentItemChanged that results from this action (to avoid infinite recursion) - currencyManager.AddNew(); - try + OnDataError(ex); + } + + if (_parentManager is CurrencyManager currencyManager) + { + if (currencyManager.Count > 0) { - IgnoreItemChangedTable.Add(currencyManager); - currencyManager.CancelCurrentEdit(); + // Parent list has a current row, so get the related list from the relevant property on that row. + SetDataSource(_fieldInfo.GetValue(currencyManager.Current)); + listposition = (Count > 0 ? 0 : -1); } - finally + else { - if (IgnoreItemChangedTable.Contains(currencyManager)) + // APPCOMPAT: bring back the Everett behavior where the currency manager adds an item and + // then it cancels the addition. + // + // really, really hocky. + // will throw if the list in the curManager is not IBindingList + // and this will fail if the IBindingList does not have list change notification. read on.... + // when a new item will get added to an empty parent table, + // the table will fire OnCurrentChanged and this method will get executed again + // allowing us to set the data source to an object with the right properties (so we can show + // metadata at design time). + // we then call CancelCurrentEdit to remove the dummy row, but making sure to ignore any + // OnCurrentItemChanged that results from this action (to avoid infinite recursion) + currencyManager.AddNew(); + try + { + IgnoreItemChangedTable.Add(currencyManager); + currencyManager.CancelCurrentEdit(); + } + finally { - IgnoreItemChangedTable.Remove(currencyManager); + if (IgnoreItemChangedTable.Contains(currencyManager)) + { + IgnoreItemChangedTable.Remove(currencyManager); + } } } } - } - else - { - // Case where the parent is not a list, but a single object - SetDataSource(_fieldInfo.GetValue(_parentManager.Current)); - listposition = (Count > 0 ? 0 : -1); - } + else + { + // Case where the parent is not a list, but a single object + SetDataSource(_fieldInfo.GetValue(_parentManager.Current)); + listposition = (Count > 0 ? 0 : -1); + } + + if (oldlistposition != listposition) + { + OnPositionChanged(EventArgs.Empty); + } - if (oldlistposition != listposition) + OnCurrentChanged(EventArgs.Empty); + OnCurrentItemChanged(EventArgs.Empty); + } + finally { - OnPositionChanged(EventArgs.Empty); + _handlingCurrentItemChanged = false; } - - OnCurrentChanged(EventArgs.Empty); - OnCurrentItemChanged(EventArgs.Empty); } } diff --git a/src/test/unit/System.Windows.Forms/System/Windows/Forms/BindingContextTests.cs b/src/test/unit/System.Windows.Forms/System/Windows/Forms/BindingContextTests.cs index d38110dc4b0..564a5e0a736 100644 --- a/src/test/unit/System.Windows.Forms/System/Windows/Forms/BindingContextTests.cs +++ b/src/test/unit/System.Windows.Forms/System/Windows/Forms/BindingContextTests.cs @@ -958,6 +958,35 @@ public void BindingContext_UpdateBinding_NullBindingContext_Success() Assert.Null(binding.BindingManagerBase); } + [Fact] + public void BindingContext_UpdateBinding_RelatedManagerCurrentItemChangedDuringPullData_DoesNotReenter() + { + BindingContext context = []; + BindingList parentList = null; + ReentrantChildDataSource child = new(() => parentList.ResetItem(0)); + ReentrantParentDataSource parent = new(child); + parentList = [parent]; + + ReentrantBindableComponent component = new() { BindingContext = context }; + Binding binding = component.DataBindings.Add( + nameof(ReentrantBindableComponent.Value), + parentList, + $"{nameof(ReentrantParentDataSource.Children)}.{nameof(ReentrantChildDataSource.Value)}"); + + CurrencyManager relatedManager = Assert.IsAssignableFrom(context[parentList, nameof(ReentrantParentDataSource.Children)]); + Assert.Same(relatedManager, binding.BindingManagerBase); + + Exception dataError = null; + relatedManager.DataError += (sender, e) => dataError = e.Exception; + + component.Value = "updated"; + parentList.ResetItem(0); + + Assert.Null(dataError); + Assert.Equal(1, child.ValueSetCount); + Assert.Equal("updated", child.Value); + } + [Fact] public void BindingContext_InvokeCircularWithoutComponent_ThrowsArgumentException() { @@ -1042,6 +1071,67 @@ private class ObjectDataSource public object Property { get; set; } } + private class ReentrantParentDataSource + { + public ReentrantParentDataSource(ReentrantChildDataSource child) + { + Children = [child]; + } + + public BindingList Children { get; } + } + + private class ReentrantChildDataSource + { + readonly Action valueSetCallback; + string value = "initial"; + + public ReentrantChildDataSource(Action valueSetCallback) + { + this.valueSetCallback = valueSetCallback; + } + + public int ValueSetCount { get; private set; } + + public string Value + { + get => value; + set + { + ValueSetCount++; + if (ValueSetCount > 1) + { + throw new InvalidOperationException("Related manager pulled data re-entrantly."); + } + + this.value = value; + valueSetCallback(); + } + } + } + + private class ReentrantBindableComponent : BindableComponent + { + string value; + + public string Value + { + get => value; + set + { + if (this.value == value) + { + return; + } + + this.value = value; + ValueChanged?.Invoke(this, EventArgs.Empty); + } + } + + public event EventHandler ValueChanged; + } + private class SubBindingContext : BindingContext { public new void Add(object dataSource, BindingManagerBase listManager) => base.Add(dataSource, listManager);