[Bug]: OnCompleted never fires v7.9.*

[marmy28]

Describe the bug 🐞

I used similar code as the below step with DynamicData v7.8.6. I updated to 7.9.14 and OnCompleted subscription stopped firing.

Step to reproduce

// Arrange
using var disposable = new CompositeDisposable();
var publishedObservable = Observable.Create(o =>
{
o.OnNext(1);
o.OnCompleted();
return Disposable.Empty;
})
.ToObservableChangeSet(i => i)
.Publish();
var hasCompleted = false;

// Act
var d = publishedObservable
.Subscribe(
_ => { },
_ => { },
() => { hasCompleted = true; });
disposable.Add(publishedObservable.Connect());
disposable.Add(d);

// Assert
Assert.IsTrue(hasCompleted);

Reproduction repository

No response

Expected behavior

With v7.8.6 this unit test passes. With v7.9.1 and above OnCompleted never triggers in the subscribe.

Screenshots 🖼️

No response

IDE

Visual Studio 2022

Operating system

Windows

Version

10

Device

Laptop

DynamicData Version

7.9.14

Additional information ℹ️

No response

[RolandPheasant]

Thanks for letting my know. I will take a look as soon as I can.

[JakenVeina]

The core problem is that the internal .Subscribe() call that subscribes to the source supplies an onNext and onError handler, but just ignores onCompleted notifications. That's easy enough to fix for the trivial scenario, we just need to pass along observer.OnCompleted, but when expiration or size-limit parameters are applied, we actually don't WANT to complete the downstream, when the upstream completes, we want to keep it open until any outstanding remove operations finish. I tried a couple things, but I'm not seeing a clean way to do this with the current implementation.

Unless there's any objections, I'm going to work on enhancing the test coverage for this operator, which currently consists of only one test, and then once that's accepted and merged, re-write the operator from scratch, as there's also opportunity for performance improvement at the same time.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.