[Bug]: TransformAsync on IChangeSet drops events

[FilippoPolo]

Describe the bug 🐞

TransformAsync on IChangeSet can silently drop events from the output observable. Specifically, it looks like in a change set that has both Remove and Add events, the Remove event is dropped.

Step to reproduce

The following test reproduces the problem. It uses a dummy transform. The last Assert fails.

public class TR
{
    public long ID { get; set; }
    public string Name { get; set; } = string.Empty;
}

[Fact(Timeout = 5000)]
public async Task TransformAsyncTest()
{
    var trA = new TR() { ID = 1, Name = "Foo" };
    var trB = new TR() { ID = 1, Name = "Bar" };
    List<ChangeSet<TR>> changes = [
        new ChangeSet<TR>([
            new Change<TR>(ListChangeReason.Add, trA )]),
        new ChangeSet<TR>([
            new Change<TR>(ListChangeReason.Remove, trA ),
            new Change<TR>(ListChangeReason.Add, trB )])];
    var observable = changes.ToObservable();

    var changesSync = new List<IChangeSet<TR>>();
    using var _1 = observable.Subscribe(changesSync.Add);
    Assert.Equal(2, changesSync.Count);
    Assert.Equal(1, changesSync[0].Count);
    Assert.Equal(2, changesSync[1].Count);

    var changesAsync = new List<IChangeSet<TR>>();
    static async Task<TR> transform(TR rec, Optional<TR> existing) => rec;
    using var _2 = observable.TransformAsync<TR, TR>(transform).Subscribe(changesAsync.Add);
    Assert.Equal(2, changesAsync.Count);
    Assert.Equal(1, changesAsync[0].Count);
    Assert.Equal(2, changesAsync[1].Count);
}

Reproduction repository

https://github.com/FilippoPolo/DynamicData

Expected behavior

I would expect a dummy transform not to change the output.

Screenshots 🖼️

No response

IDE

No response

Operating system

No response

Version

No response

Device

No response

DynamicData Version

9.4.33

Additional information ℹ️

No response

[JakenVeina]

It's actually got nothing to do with the sequencing of changes, it emerges for Remove changes with an unspecified index. I've confirmed that the defect does not present when adding in appropriate indexes to your sample, or just rewriting the whole thing to have a SourceList<T> generate the changes.

The faulty logic can be seen here.

var toRemove = transformed.FirstOrDefault(t => ReferenceEquals(t.Source, t));

if (toRemove is not null)
{
    transformed.Remove(toRemove);
}

As far as I can tell, this is just a typo that's been around since the operator was introduced in 2017, cause the logic makes NO goddamn sense. We're attempting to lookup the item to be removed by looking for an item where the source (I.E. the un-transformed item) is equal to a container object that houses un-transformed/transformed item pairs, and doesn't even exist until after the transform completes? Apparently, you're the first one to run into this specific scenario of using .TransformAsync() upon un-indexed changes in the 9 years this has been around, or at least you're the first one to care enough to report it.

Not sure what your real-world scenario is that you ended up with list changes without indexing, but fixing that would be the immediate workaround for you to implement (plus, it'll be more performant). I'll ping you when we can get a proper fix deployed.

[FilippoPolo]

Thanks! The scenario is that I have an interface whose type is IEnumerable<IChangeSet>. One of the implementations actually has keys, but the interface doesn't want to know about that, so I call RemoveKey() right before returning. Later, the consumer wants to transform asynchronously and bind, and that's where the problem manifests. I'll see if I can rework the logic.