Skip to content

Commit 8db4806

Browse files
authored
Rewrote the list variant of the OnItemRemoved() operator, in accordance with #1014, as a fix for #1062. (#1067)
1 parent 4aaa9c1 commit 8db4806

File tree

4 files changed

+101
-81
lines changed

4 files changed

+101
-81
lines changed

src/DynamicData.Tests/List/OnItemRemovedFixture.cs

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
using System;
22
using System.Collections.Generic;
33
using System.Linq;
4-
4+
using System.Reactive.Linq;
5+
using System.Reactive.Subjects;
56
using FluentAssertions;
67
using Xunit;
78

@@ -11,6 +12,29 @@ namespace DynamicData.Tests.List;
1112

1213
public class OnItemRemovedFixture
1314
{
15+
// https://github.com/reactivemarbles/DynamicData/issues/1061
16+
[Theory]
17+
[InlineData(true)]
18+
[InlineData(false)]
19+
public void SubscriberDoesNotHandleErrors_ErrorBubblesUpstream(bool invokeOnUnsubscribe)
20+
{
21+
using var source = new TestSourceList<int>();
22+
23+
using var subscription = source.Connect()
24+
.OnItemRemoved(
25+
removeAction: static _ => { },
26+
invokeOnUnsubscribe: invokeOnUnsubscribe)
27+
.Subscribe();
28+
29+
var error = new Exception("Test");
30+
31+
FluentActions.Invoking(() => source.SetError(error))
32+
.Should()
33+
.Throw<Exception>("errors not handled by the subscriber should propagate upstream to the caller")
34+
.Which
35+
.Should().BeSameAs(error);
36+
}
37+
1438
[Theory]
1539
[InlineData(0, 0, 0)]
1640
[InlineData(1, 0, 0)]

src/DynamicData/List/Internal/OnBeingRemoved.cs

Lines changed: 0 additions & 61 deletions
This file was deleted.
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
// Copyright (c) 2011-2025 Roland Pheasant. All rights reserved.
2+
// Roland Pheasant licenses this file to you under the MIT license.
3+
// See the LICENSE file in the project root for full license information.
4+
5+
using System.Reactive.Linq;
6+
7+
namespace DynamicData.List.Internal;
8+
9+
internal static class OnItemRemoved<T>
10+
where T : notnull
11+
{
12+
public static IObservable<IChangeSet<T>> Create(
13+
IObservable<IChangeSet<T>> source,
14+
Action<T> removeAction,
15+
bool invokeOnUnsubscribe)
16+
{
17+
source.ThrowArgumentNullExceptionIfNull(nameof(source));
18+
removeAction.ThrowArgumentNullExceptionIfNull(nameof(removeAction));
19+
20+
var removalProcessor = source.Do(changeSet =>
21+
{
22+
foreach (var change in changeSet)
23+
{
24+
switch (change.Reason)
25+
{
26+
case ListChangeReason.Clear:
27+
case ListChangeReason.RemoveRange:
28+
foreach (var item in change.Range)
29+
removeAction.Invoke(item);
30+
break;
31+
32+
case ListChangeReason.Remove:
33+
removeAction.Invoke(change.Item.Current);
34+
break;
35+
36+
case ListChangeReason.Replace:
37+
removeAction.Invoke(change.Item.Previous.Value);
38+
break;
39+
}
40+
}
41+
});
42+
43+
return invokeOnUnsubscribe
44+
? Observable.Create<IChangeSet<T>>(observer =>
45+
{
46+
var items = new List<T>();
47+
48+
return removalProcessor
49+
.Do(changeSet => items.Clone(changeSet))
50+
.Finally(() =>
51+
{
52+
foreach (var item in items)
53+
removeAction.Invoke(item);
54+
})
55+
.SubscribeSafe(observer);
56+
})
57+
: removalProcessor;
58+
}
59+
}

src/DynamicData/List/ObservableListEx.cs

Lines changed: 17 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1276,26 +1276,24 @@ public static IObservable<IChangeSet<T>> OnItemRefreshed<T>(
12761276
refreshAction: refreshAction);
12771277

12781278
/// <summary>
1279-
/// Callback for each item as and when it is being removed from the stream.
1279+
/// Invokes a given action for every item removed from the source list stream.
12801280
/// </summary>
1281-
/// <typeparam name="T">The type of the object.</typeparam>
1282-
/// <param name="source">The source.</param>
1283-
/// <param name="removeAction">The remove action.</param>
1284-
/// <param name="invokeOnUnsubscribe"> Should the remove action be invoked when the subscription is disposed.</param>
1285-
/// <returns>An observable which emits the change set.</returns>
1286-
/// <exception cref="ArgumentNullException">
1287-
/// source
1288-
/// or
1289-
/// removeAction.
1290-
/// </exception>
1291-
public static IObservable<IChangeSet<T>> OnItemRemoved<T>(this IObservable<IChangeSet<T>> source, Action<T> removeAction, bool invokeOnUnsubscribe = true)
1292-
where T : notnull
1293-
{
1294-
source.ThrowArgumentNullExceptionIfNull(nameof(source));
1295-
removeAction.ThrowArgumentNullExceptionIfNull(nameof(removeAction));
1296-
1297-
return new OnBeingRemoved<T>(source, removeAction, invokeOnUnsubscribe).Run();
1298-
}
1281+
/// <typeparam name="T">The type of items in the list.</typeparam>
1282+
/// <param name="source">The list stream whose items are to be passed to <paramref name="removeAction"/>.</param>
1283+
/// <param name="removeAction">The action to invoke upon each removed item.</param>
1284+
/// <param name="invokeOnUnsubscribe">A flag indicating whether <paramref name="removeAction"/> should be invoked upon teardown of the stream. This includes disposal of subscriptions, completion notifications, and error notifications.</param>
1285+
/// <returns>A list stream, containing all items in <paramref name="source"/>, with changes published after <paramref name="removeAction"/> has been invoked.</returns>
1286+
/// <exception cref="ArgumentNullException">Throws for <paramref name="source"/> and <paramref name="removeAction"/>.</exception>
1287+
/// <remarks>Note that "removed" items includes items from <see cref="ListChangeReason.Remove"/>, <see cref="ListChangeReason.RemoveRange"/>, <see cref="ListChangeReason.Replace"/>, and <see cref="ListChangeReason.Clear"/> changes.</remarks>
1288+
public static IObservable<IChangeSet<T>> OnItemRemoved<T>(
1289+
this IObservable<IChangeSet<T>> source,
1290+
Action<T> removeAction,
1291+
bool invokeOnUnsubscribe = true)
1292+
where T : notnull
1293+
=> List.Internal.OnItemRemoved<T>.Create(
1294+
source: source,
1295+
removeAction: removeAction,
1296+
invokeOnUnsubscribe: invokeOnUnsubscribe);
12991297

13001298
/// <summary>
13011299
/// Apply a logical Or operator between the collections.

0 commit comments

Comments
 (0)