TransformOnObservable mixes the order of changes in a given change set?

[paagamelo2]

Ciao guys

I've been observing a weird behavior with TransformOnObservable. Namely, it mixes up the order of changes in a given change set.

This is a minimal test reproducing the issue:

private record Person(string Name, int Age);

[Test]
public void TransformOnObservableMixesTheOrderOfChangesInAGivenChangeSet()
{
    var sourceCache = new SourceCache<Person, string>(person => person.Name);

    // Cache emitted change sets
    var observedChangeSets = new List<IChangeSet<Person, string>>();
    using var subscription = sourceCache
        .AsObservableCache()
        .Connect()
        // Dummy observable that just returns the person
        .TransformOnObservable(Observable.Return)
        .Subscribe(changeSet =>
        {
            observedChangeSets.Add(changeSet);
        });

    sourceCache.Edit(innerCache =>
    {
        innerCache.AddOrUpdate(new Person("John", 30));
        innerCache.AddOrUpdate(new Person("Jane", 25));
        innerCache.AddOrUpdate(new Person("Doe", 40));
    });

    sourceCache.Edit(innerCache =>
    {
        innerCache.Clear();
        innerCache.AddOrUpdate(new Person("John", 32));
        innerCache.AddOrUpdate(new Person("Jane", 29));
        innerCache.AddOrUpdate(new Person("Doe", 18));
    });

    var lastChangeSet = observedChangeSets.Last();
    Debugger.Break();
}

In lastChangeSet I'd expect to find 3 Removes followed by 3 Adds. Instead, there are 3 Updates followed by 3 Removes; as if the AddOrUpdates were called before the Clear somehow.

Clearly, innerCache.Clear() makes little sense in this case. However, I think there might be real-life scenarios where an operation like that is legit. For instance, suppose sourceCache is representing the people living in a particular city selected by the user. If the user changes the city of interest, we may want to clear the cache and add people from the new city. If the key doesn't take into account the city (therefore allowing people from different cities to have the same key), any new people whose key was already in the cache will be incorrectly removed.

Currently, we're working around this by using EditDiff rather than Edit, which will emit an update for any item with the same key. But I'd be curious to know if this is a bug or whether I'm misusing Dynamic Data in any way.

Thanks
Lorenzo

[JakenVeina]

Yeah, I'm definitely gonna classify this as a straight-up defect. Bottom line, if you drop .ToCollection() after .TransformOnObservable(), the output is just wrong. Given that your transform is unity, the output collection should always be identical to the source collection, and with the second change you're making there, that doesn't happen. The output emits an empty collection when there's 3 items in the source.

Doing a clear and then re-adding items with some of the same keys, like you have here might be a niche scenario, but I can entirely imagine a valid one. More generally, if the API allows you to do it, there would have to be a VERY compelling, and intuitive reason for me to say it shouldn't be supported.

Looking at the operator, innards, I think it's an issue with ordering of separate subscriptions. The operator does a .Publish() on source so it can apply two separate subscriptions to it: one that does a .MergeMany() to get all the item-level changes, and a separate one that tracks the parent streams of each item, to handle key removals. So, yes, this has the end result of not processing the changes in the order they come in. It processes all adds, updates, and child emissions, then processes all removes, since the Subject that's buried in the publish does, effectively, a foreach over the two subscribers, then each subscriber iterates over the changes for itself.

This might be a good opportunity to re-write and re-optimize the whole operator, as we've done for some others recently. I'm not sure if there's an easy fix that doesn't involve getting rid of the whole two-subscriptions concept, in favor of a single subscription.

@RolandPheasant @dwcullop Any objection to a rewrite? Or is there a simpler option I'm not seeing?

[RolandPheasant]

@JakenVeina no objection at all. It's fundamental to DD that the order is always preserved for edit operations.

[JakenVeina]

@dwcullop Actually, this is an operator you wrote just last year, I'd forgotten it's that new. Do you mind a rewrite, or would you like a shot at this one?

[dwcullop]

The nature of this operator is that it can't always preserve the order of the changes. It can't emit an add event until the transformation observable fires at least one value, which could happen in any order.

However, when it is completely synchronous like this example, it should be able to. I created a bug (#1007) and assigned it to myself.