diff --git a/.github/instructions/dynamicdata-cache.instructions.md b/.github/instructions/dynamicdata-cache.instructions.md index 45b1874bb..71e20871d 100644 --- a/.github/instructions/dynamicdata-cache.instructions.md +++ b/.github/instructions/dynamicdata-cache.instructions.md @@ -826,6 +826,8 @@ Filters Update changes based on reference equality or a custom predicate. If fil | `WhenValueChanged(expr)` | Like above but emits just the property value (no sender). | | `WhenAnyPropertyChanged()` | Emits the item when **any** property changes (no specific property). | +Property paths used by `WhenPropertyChanged` and `WhenValueChanged` evaluate numeric conversions before subsequent property access. + --- ## Writing a New Cache Operator diff --git a/.github/instructions/dynamicdata-list.instructions.md b/.github/instructions/dynamicdata-list.instructions.md index 1f969b3ff..469b05a75 100644 --- a/.github/instructions/dynamicdata-list.instructions.md +++ b/.github/instructions/dynamicdata-list.instructions.md @@ -500,6 +500,8 @@ myObservable.ToObservableChangeSet(expireAfter: item => TimeSpan.FromMinutes(5)) ### Property Observation +Property paths used by `WhenPropertyChanged` and `WhenValueChanged` evaluate numeric conversions before subsequent property access. + ```csharp // Observe a property on all items (requires INotifyPropertyChanged) list.Connect() diff --git a/src/DynamicData.Tests/Binding/WhenPropertyChangedBehaviorFixture.Conversions.cs b/src/DynamicData.Tests/Binding/WhenPropertyChangedBehaviorFixture.Conversions.cs new file mode 100644 index 000000000..2f12c89d8 --- /dev/null +++ b/src/DynamicData.Tests/Binding/WhenPropertyChangedBehaviorFixture.Conversions.cs @@ -0,0 +1,131 @@ +// Copyright (c) 2011-2025 Roland Pheasant. All rights reserved. +// Roland Pheasant 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 DynamicData.Binding; +using DynamicData.Tests.Domain; +using DynamicData.Tests.Utilities; + +using FluentAssertions; + +using Xunit; + +namespace DynamicData.Tests.Binding; + +public sealed partial class WhenPropertyChangedBehaviorFixture +{ + /// Verifies that a numeric conversion is evaluated before reading a property of the converted value. + [Fact] + public void NumericConversionBeforePropertyAccess_InitialValue_IsObserved() + { + // Arrange + // An odd numerator produces an exactly representable, non-integral amount. + var amount = (_randomizer.Int(1, ushort.MaxValue) | 1) / 4d; + var model = new ObservablePrice { Amount = amount }; + var expectedScale = ((decimal)amount).Scale; + + // Act + using var subscription = model.WhenValueChanged(static price => ((decimal)price.Amount).Scale) + .RecordValues(out var results); + + // Assert + results.Error.Should().BeNull(because: "the next property belongs to the converted decimal, not the source double"); + results.RecordedValues.Should().Equal(new[] { expectedScale }, because: "the initial value must match the supplied expression"); + results.HasCompleted.Should().BeFalse(because: "further amount changes remain observable"); + } + + /// Verifies that property changes remain observable through a value-changing numeric conversion. + /// Whether subscribing requests an initial value notification. + [Theory] + [InlineData(false)] + [InlineData(true)] + public void NumericConversionBeforePropertyAccess_PropertyChanges_AreObserved(bool notifyOnInitialValue) + { + // Arrange + // Odd quarters and eighths have distinct decimal scales without floating-point rounding. + var initialAmount = (_randomizer.Int(1, ushort.MaxValue) | 1) / 4d; + var changedAmount = (_randomizer.Int(1, ushort.MaxValue) | 1) / 8d; + var model = new ObservablePrice { Amount = initialAmount }; + var initialScale = ((decimal)initialAmount).Scale; + var changedScale = ((decimal)changedAmount).Scale; + var expectedScales = notifyOnInitialValue ? new[] { initialScale, changedScale } : new[] { changedScale }; + using var subscription = model.WhenValueChanged(static price => ((decimal)price.Amount).Scale, notifyOnInitialValue) + .RecordValues(out var results); + + // Act + model.Amount = changedAmount; + + // Assert + results.Error.Should().BeNull(because: "conversion must be applied on both initial and subsequent chain reads"); + results.RecordedValues.Should().Equal(expectedScales, because: "notifications must match the expression and initial-value option"); + results.HasCompleted.Should().BeFalse(because: "amount changes do not complete the observation"); + } + + /// Verifies that a numeric conversion at the end of a property path preserves initial and changed values. + [Fact] + public void NumericConversionAtLeaf_PropertyChanges_AreObserved() + { + // Arrange + var initialAmount = _randomizer.Double(); + var changedAmount = initialAmount + _randomizer.Double(1, 2); + var model = new ObservablePrice { Amount = initialAmount }; + var expectedAmounts = new[] { (decimal)initialAmount, (decimal)changedAmount }; + using var subscription = model.WhenValueChanged(static price => (decimal)price.Amount) + .RecordValues(out var results); + + // Act + model.Amount = changedAmount; + + // Assert + results.Error.Should().BeNull(because: "a conversion must also remain supported as the final expression step"); + results.RecordedValues.Should().Equal(expectedAmounts, because: "each observed value must be converted to the requested type"); + results.HasCompleted.Should().BeFalse(because: "further amount changes remain observable"); + } + + /// Verifies that a reference cast preserves observation of properties on the runtime model. + [Fact] + public void ReferenceCastBeforePropertyAccess_PropertyChanges_AreObserved() + { + // Arrange + var person = Fakers.Person.Clone().WithSeed(_randomizer).Generate(); + INotifyPropertyChanged model = person; + var initialAge = person.Age; + var changedAge = initialAge + _randomizer.Int(1, byte.MaxValue); + using var subscription = model.WhenValueChanged(static source => ((Person)source).Age) + .RecordValues(out var results); + + // Act + person.Age = changedAge; + + // Assert + results.Error.Should().BeNull(because: "supported reference casts must preserve property observation"); + results.RecordedValues.Should().Equal(new[] { initialAge, changedAge }, because: "the runtime model remains the notification source"); + results.HasCompleted.Should().BeFalse(because: "further age changes remain observable"); + } + + /// Verifies that an interface cast preserves observation after replacing an intermediate object. + [Fact] + public void InterfaceCastBeforePropertyAccess_ReplacementChildChanges_AreObserved() + { + // Arrange + var initialAge = _randomizer.Int(1, byte.MaxValue); + var replacementAge = initialAge + _randomizer.Int(1, byte.MaxValue); + var changedAge = replacementAge + _randomizer.Int(1, byte.MaxValue); + var parent = new ParentModel { Child = new ChildModel { Age = initialAge } }; + var replacement = new ChildModel { Age = replacementAge }; + using var subscription = parent.WhenValueChanged(static source => ((IHasAge)source.Child!).Age) + .RecordValues(out var results); + parent.Child = replacement; + + // Act + replacement.Age = changedAge; + + // Assert + results.Error.Should().BeNull(because: "supported interface casts must preserve nested property observation"); + results.RecordedValues.Should().Equal(new[] { initialAge, replacementAge, changedAge }, + because: "the interface property must follow the replacement child and its subsequent changes"); + results.HasCompleted.Should().BeFalse(because: "further child changes remain observable"); + } +} diff --git a/src/DynamicData.Tests/Binding/WhenPropertyChangedBehaviorFixture.cs b/src/DynamicData.Tests/Binding/WhenPropertyChangedBehaviorFixture.cs index 718df9537..919eaef79 100644 --- a/src/DynamicData.Tests/Binding/WhenPropertyChangedBehaviorFixture.cs +++ b/src/DynamicData.Tests/Binding/WhenPropertyChangedBehaviorFixture.cs @@ -6,20 +6,35 @@ using System.Collections.Generic; using System.ComponentModel; using System.Linq; + +using Bogus; + using DynamicData.Binding; using DynamicData.Tests.Utilities; using FluentAssertions; using Xunit; +using Xunit.Abstractions; namespace DynamicData.Tests.Binding; /// /// Single-threaded contract tests for : -/// handler attachment ordering, no-dedup semantics, deep-chain re-walks on swaps. +/// handler attachment ordering, expression conversions, no-dedup semantics, deep-chain re-walks on swaps. /// -public sealed class WhenPropertyChangedBehaviorFixture +public sealed partial class WhenPropertyChangedBehaviorFixture { + private readonly Randomizer _randomizer; + + /// Initializes deterministic inputs for property-observation contracts. + /// Receives the seed used to generate test inputs. + public WhenPropertyChangedBehaviorFixture(ITestOutputHelper output) + { + const int seed = 0x35C1_709B; + _randomizer = new Randomizer(seed); + output.WriteLine($"{nameof(WhenPropertyChangedBehaviorFixture)} seed: 0x{seed:X8}"); + } + [Fact] public void Shallow_NotifyInitialFalse_SubscribesHandlerBeforeReturning() { @@ -204,6 +219,24 @@ public int Value } } + /// An observable model with a numeric property, used to exercise value-changing conversions. + private sealed class ObservablePrice : INotifyPropertyChanged + { + private double _amount; + + public event PropertyChangedEventHandler? PropertyChanged; + + public double Amount + { + get => _amount; + set + { + _amount = value; + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(Amount))); + } + } + } + private sealed class ParentModel : INotifyPropertyChanged { private ChildModel? _child; diff --git a/src/DynamicData/Binding/ExpressionBuilder.cs b/src/DynamicData/Binding/ExpressionBuilder.cs index a34bf3e14..75aa6e610 100644 --- a/src/DynamicData/Binding/ExpressionBuilder.cs +++ b/src/DynamicData/Binding/ExpressionBuilder.cs @@ -53,11 +53,19 @@ internal static Func> CreatePropertyChangedFactory(thi return property.GetValue; - // I.E. cast operations. Since we're just dealing with everything as `object`, there's really nothing for us - // to do. If we cast from `object` to the desired type, we'll just immediately cast back to `object` to - // return. The runtime has to resolve and do the correct cast anyway, regardless of what we do here. - case UnaryExpression { NodeType: ExpressionType.Convert }: - return static target => target; + case UnaryExpression { NodeType: ExpressionType.Convert } conversion: + // Built-in reference casts and boxing/unboxing preserve a non-null chain target. + // Numeric and user-defined conversions must produce the target used by the next step. + if (conversion.Method is null && (!conversion.Operand.Type.IsValueType || !conversion.Type.IsValueType)) + { + return static target => target; + } + + var parameter = Expression.Parameter(typeof(object), "target"); + var operand = Expression.Convert(parameter, conversion.Operand.Type); + var converted = conversion.Update(operand); + + return Expression.Lambda>(Expression.Convert(converted, typeof(object)), parameter).Compile(); case null: throw new ArgumentNullException(nameof(source)); diff --git a/src/DynamicData/Binding/NotifyPropertyChangedEx.cs b/src/DynamicData/Binding/NotifyPropertyChangedEx.cs index 3c3ac0924..5a71480d3 100644 --- a/src/DynamicData/Binding/NotifyPropertyChangedEx.cs +++ b/src/DynamicData/Binding/NotifyPropertyChangedEx.cs @@ -245,6 +245,11 @@ public static class NotifyPropertyChangedEx /// For an object like Parent.Child.Sibling, sibling is an object so if Child is null, the value null and obtainable and is returned as null. /// A observable which also notifies when the property value changes. /// propertyAccessor. + /// + /// Property paths can contain reference and numeric conversions. Value-changing conversions are evaluated + /// before accessing subsequent properties in the path. + /// + /// public static IObservable> WhenPropertyChanged(this TObject source, Expression> propertyAccessor, bool notifyOnInitialValue = true, Func? fallbackValue = null) where TObject : INotifyPropertyChanged { @@ -267,6 +272,10 @@ public static IObservable> WhenPropertyChanged /// For example when observing Parent.Child.Age, if Child is null the value is unobtainable as Age is a struct and cannot be set to Null. /// For an object like Parent.Child.Sibling, sibling is an object so if Child is null, the value null and obtainable and is returned as null. /// An observable which emits the results. + /// + /// Supports the property-path conversions described by . + /// + /// public static IObservable WhenValueChanged(this TObject source, Expression> propertyAccessor, bool notifyOnInitialValue = true, Func? fallbackValue = null) where TObject : INotifyPropertyChanged {