diff --git a/src/ReactiveUI.Binding.Shared/ObservableForProperty/ExpressionChainSink.cs b/src/ReactiveUI.Binding.Shared/ObservableForProperty/ExpressionChainSink.cs index 73d56847..e012499b 100644 --- a/src/ReactiveUI.Binding.Shared/ObservableForProperty/ExpressionChainSink.cs +++ b/src/ReactiveUI.Binding.Shared/ObservableForProperty/ExpressionChainSink.cs @@ -305,6 +305,13 @@ private sealed class Level : IDisposable /// This link's index/argument array (non-null only for indexer links), cached once. private readonly object?[]? _arguments; + /// Whether this level is subscribing to its link's notifications. + /// + /// Only read under the gate, so only a notification the subscribing thread raises from inside + /// Subscribe sees it set. + /// + private bool _attaching; + /// Initializes a new instance of the class. /// The owning chain sink. /// This watcher's position in the chain. @@ -341,10 +348,14 @@ public void SetParent(object? parent) // Subscribe before reading, so a change between the two is reported rather than lost. The // caller holds the gate, so a notification that races this window queues behind it and - // re-reports the value the kicker is about to push; the sink drops that one repeat. + // re-reports the value the kicker is about to push; the sink drops that one repeat. A + // notification raised from inside Subscribe, such as the one a POCO link emits, is ignored: + // the kicker reads the same value straight after. + _attaching = true; _subscription.Disposable = ReactiveNotifyPropertyChangedMixins .NotifyForProperty(parent, link, _sink._beforeChange, _sink._suppressWarnings) .Subscribe(new Observer(this)); + _attaching = false; Push(ReadValue(parent), fromKicker: true); } @@ -359,7 +370,7 @@ private void OnNotification(IObservedChange change) { lock (_sink._gate) { - if (_sink._disposed) + if (_sink._disposed || _attaching) { return; } diff --git a/src/tests/ReactiveUI.Binding.Tests/Mixins/PocoLinkUnsafeBindingTests.cs b/src/tests/ReactiveUI.Binding.Tests/Mixins/PocoLinkUnsafeBindingTests.cs new file mode 100644 index 00000000..bc9b1340 --- /dev/null +++ b/src/tests/ReactiveUI.Binding.Tests/Mixins/PocoLinkUnsafeBindingTests.cs @@ -0,0 +1,85 @@ +// Copyright (c) 2019-2026 ReactiveUI and Contributors. All rights reserved. +// ReactiveUI and Contributors licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +using System.ComponentModel; +using ReactiveUI.Binding.Tests.Fallback; +using ReactiveUI.Binding.Tests.TestModels; + +namespace ReactiveUI.Binding.Tests.Mixins; + +/// +/// A runtime view-first binding whose view raises no change notification reads the view through links that emit +/// once on subscribe. The binding still sees one initial value per side. +/// +[NotInParallel] +public class PocoLinkUnsafeBindingTests +{ + /// The value the view model starts out holding. + private const string ViewModelValue = "model"; + + /// The value typed into the view. + private const string EditedValue = "edited"; + + /// Creating a two-way binding writes the view model's value to the view and never back. + /// A task representing the asynchronous test operation. + [Test] + public async Task BindUnsafe_ViewWithoutNotification_DoesNotWriteBackOnCreate() + { + RuntimeObservationFallbackTests.EnsureInitialized(); + var viewModel = new CountingViewModel { Name = ViewModelValue }; + var view = new SilentView { ViewModel = viewModel }; + viewModel.NameWrites = 0; + + using var binding = view.BindUnsafe(viewModel, static vm => vm.Name, static v => v.Editor.IsNotNullString); + var writesOnCreate = viewModel.NameWrites; + var viewOnCreate = view.Editor.IsNotNullString; + view.Editor.IsNotNullString = EditedValue; + + using (Assert.Multiple()) + { + await Assert.That(writesOnCreate).IsEqualTo(0); + await Assert.That(viewOnCreate).IsEqualTo(ViewModelValue); + await Assert.That(viewModel.Name).IsEqualTo(EditedValue); + } + } + + /// A view model that counts the writes to its bound property. + internal sealed class CountingViewModel : INotifyPropertyChanged + { + /// + public event PropertyChangedEventHandler? PropertyChanged; + + /// Gets or sets the number of times has been written. + public int NameWrites { get; set; } + + /// Gets or sets the bound name. + public string Name + { + get; + set + { + NameWrites++; + field = value; + PropertyChanged?.Invoke(this, new(nameof(Name))); + } + } = string.Empty; + } + + /// A view that raises no change notification and holds a control that does. + internal sealed class SilentView : IViewFor + { + /// + public CountingViewModel? ViewModel { get; set; } + + /// + object? IViewFor.ViewModel + { + get => ViewModel; + set => ViewModel = (CountingViewModel?)value; + } + + /// Gets the notifying control the binding writes to. + public TestFixture Editor { get; } = new(); + } +} diff --git a/src/tests/ReactiveUI.Binding.Tests/TestModels/PocoChainRoot.cs b/src/tests/ReactiveUI.Binding.Tests/TestModels/PocoChainRoot.cs new file mode 100644 index 00000000..3b9224ed --- /dev/null +++ b/src/tests/ReactiveUI.Binding.Tests/TestModels/PocoChainRoot.cs @@ -0,0 +1,12 @@ +// Copyright (c) 2019-2026 ReactiveUI and Contributors. All rights reserved. +// ReactiveUI and Contributors licenses this file to you under the MIT license. +// See the LICENSE file in the project root for full license information. + +namespace ReactiveUI.Binding.Tests.TestModels; + +/// A root with no change notification that holds an object which notifies. +public class PocoChainRoot +{ + /// Gets the notifying object at the end of the chain. + public TestFixture Leaf { get; } = new(); +} diff --git a/src/tests/ReactiveUI.Binding.Tests/WhenAny/ExpressionChainTests.cs b/src/tests/ReactiveUI.Binding.Tests/WhenAny/ExpressionChainTests.cs index 7122e70b..d387c856 100644 --- a/src/tests/ReactiveUI.Binding.Tests/WhenAny/ExpressionChainTests.cs +++ b/src/tests/ReactiveUI.Binding.Tests/WhenAny/ExpressionChainTests.cs @@ -136,6 +136,66 @@ public async Task WithIsDistinct_DeduplicatesSameValues() await Assert.That(values[1]).IsEqualTo("B"); } + /// + /// A link with no change notification emits once on subscribe. The chain reads the same value straight after, + /// so it reports the initial value once, and the notifying leaf behind it still reports its changes. + /// + /// A task representing the asynchronous test operation. + [Test] + public async Task PocoLinkBeforeNotifyingLeaf_EmitsTheInitialValueOnce() + { + EnsureInitialized(); + + var root = new PocoChainRoot(); + root.Leaf.IsNotNullString = StartValue; + Expression> expr = x => x.Leaf.IsNotNullString; + var values = new List(); + + using var sub = root.SubscribeToExpressionChain( + expr.Body, + false, + false, + false) + .Select(static x => x.Value) + .Subscribe(values.Add); + var initial = values.ToArray(); + + root.Leaf.IsNotNullString = ReplacementValue; + + using (Assert.Multiple()) + { + await Assert.That(initial).IsEquivalentTo([StartValue]); + await Assert.That(values).IsEquivalentTo([StartValue, ReplacementValue]); + } + } + + /// A leaf with no change notification reports its value once, and skipping the initial value leaves nothing. + /// Whether the first value is dropped. + /// The number of values the chain reports. + /// A task representing the asynchronous test operation. + [Test] + [Arguments(false, 1)] + [Arguments(true, 0)] + public async Task PocoLeaf_ReportsTheInitialValueAtMostOnce(bool skipInitial, int expectedCount) + { + EnsureInitialized(); + + var fixture = new PocoModel { Value = StartValue }; + Expression> expr = x => x.Value; + var values = new List(); + + using var sub = fixture.SubscribeToExpressionChain( + expr.Body, + false, + skipInitial, + false, + true) + .Select(static x => x.Value) + .Subscribe(values.Add); + + await Assert.That(values.Count).IsEqualTo(expectedCount); + } + /// Verifies that null in a chain propagates correctly. /// A task representing the asynchronous test operation. [Test]