[Bug]: SortAndVirtualize emits all items if the virtual request window size is 0.

[Snailya]

Describe the bug 🐞

For some scenario that requires lazy load, an initial virtualize window size 0 is expected. However, when a virtual request has size of 0, the SortAndVirtualize will ignore the request and emit all items to downstream.

I think this is the code that caused this problem, if the size is 0, the request is ignored.

var paramsChanged = _virtualRequests.Synchronize(locker)
                    .DistinctUntilChanged()
                    // exclude dodgy params
                    .Where(parameters => parameters is { StartIndex: >= 0, Size: > 0 })
                    .Select(request =>
                    {
                        virtualParams = request;

                        // have not received the comparer yet
                        if (applicator is null) return Empty;

                        // re-apply virtual changes
                        return ApplyVirtualChanges();
                    });

Step to reproduce

1. Go to '...'
2. Click on '....'
3. Scroll down to '....'
4. See error

Reproduction repository

https://github.com/Snailya/SortAndVirtualizeTest

Expected behavior

if size 0 is allowed, the downstream should have no item. if not, a validation for the size is need to notify the coder.

Screenshots 🖼️

No response

IDE

No response

Operating system

No response

Version

No response

Device

No response

DynamicData Version

No response

Additional information ℹ️

No response

[JakenVeina]

Sorry for the delayed response. I wanted to give @RolandPheasant a chance to address it, as it was his operator, and he would know if there's a specific reason this was intentional.

For my part, though, I agree, the behavior is counter-intuitive, and unless it's documented as intentional, I'd say it's a defect. However..... it kinda IS documented as intentional? Sort of? There's other virtualization operators that explicitly state they don't accept size values of 0. Some of them are marked Obsolete, but at least one of them is not.

https://github.com/reactivemarbles/DynamicData/blob/3d72c9e539e2114440754ec216f18ad7d527ffe9/src/DynamicData/Cache/ObservableCacheEx.VirtualiseAndPage.cs#L135

Also, its equivalent in List-land.

https://github.com/reactivemarbles/DynamicData/blob/3d72c9e539e2114440754ec216f18ad7d527ffe9/src/DynamicData/List/ObservableListEx.cs#L1819

If we're going to change the behavior, we'll probably want to be consistent across the board, so I'd like to have at least @RolandPheasant weigh in on the impact of changing these old operators to now allow empty pages. From what I can tell, the impact is minimal. There's a handful of spots where we're artificially restricting ourselves to Size values being greater than 0meaning the only change is that we no longer throw exceptions where we did before. The actual core virtualizer logic only passes .Size to Enumerable.Take(), which is perfectly happy to spit out an empty sequence. Likewise, within VirtualResponse, there's no restriction placed on .Size, and that appears to be an exclusively-public API: nothing inside DynamicData uses it to do anything downstream of a virtualization operator.

I've got a preliminary PR written up that should do the trick, if we want to go forward with the change.