[Bug]: SortAndPage does not work when there is only 1 page

[insomniachi]

Describe the bug 🐞

When there is only 1 page, the sorting does not happen.
if i change the page size so that there is more than 1 page, then sorting works

var pageRequest = this.WhenAnyValue(x => x.SelectedPage).Select(page => new PageRequest(page + 1, 100));
var comparer = this.WhenAnyValue(x => x.SortBy, x => x.Order).Select(GetComparer);

_itemsCache
	.Connect()
	.RefCount()
	.SortAndPage(comparer, pageRequest)
	.Bind(out _items)
	.DisposeMany()
	.Subscribe();

Step to reproduce

See description

Reproduction repository

https://github.com/reactivemarbles/DynamicData

Expected behavior

Sorting should happen even if there is only 1 page

Screenshots 🖼️

No response

IDE

Visual Studio 2022

Operating system

Windows

Version

11

Device

No response

DynamicData Version

9.0.4

Additional information ℹ️

No response

[JakenVeina]

using var itemsCache = new SourceCache<Person, string>(static person => person.UniqueKey);

itemsCache.AddOrUpdate(new Person(
    name:   "Jake",
    age:    1));

itemsCache.AddOrUpdate(new Person(
    name:   "Darrin",
    age:    2));

itemsCache.AddOrUpdate(new Person(
    name:   "Insomniachi",
    age:    3));

using var subscription = itemsCache
    .Connect()
    .RefCount()
    .SortAndPage(
        comparerChanged:    Observable.Return(Comparer<Person>.Create(static (x, y) => x.Name.CompareTo(y.Name))),
        pageRequests:       Observable.Return(new PageRequest(page: 1, size: 100)))
    .Bind(out var items)
    .DisposeMany()
    .Subscribe();

foreach (var item in items)
    Console.WriteLine(item.ToString());

This works completely fine. Output is...

Darrin. 2
Insomniachi. 3
Jake. 1

Gonna need more detail.

[insomniachi]

Here's my view model

public partial class LibraryViewModel : ObservableObject, INavigationAware
{
	private readonly IJellyfinClient _jellyfinClient;
	private readonly SourceCache<BaseItemDto, Guid> _itemsCache = new(x => x.Id ?? Guid.Empty);
	private readonly ReadOnlyObservableCollection<BaseItemDto> _items;


	public LibraryViewModel(IJellyfinClient jellyfinClient, int pageSize)
	{
		PageSize = pageSize;
		_jellyfinClient = jellyfinClient;

		var pageRequest = this.WhenAnyValue(x => x.SelectedPage).Select(page => new PageRequest(page + 1, PageSize));
		var comparer = this.WhenAnyValue(x => x.SortBy, x => x.Order)
						   .Select(GetComparer);

		_itemsCache
			.Connect()
			.RefCount()
			.SortAndPage(comparer, pageRequest)
			.Bind(out _items)
			.Subscribe();
	}

	private IComparer<BaseItemDto> GetComparer((ItemSortBy, SortOrder) sortDescription)
	{
		return sortDescription switch
		{
			(ItemSortBy.Name, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.Name ?? ""),
			(ItemSortBy.CommunityRating, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.CommunityRating ?? 0),
			(ItemSortBy.CriticRating, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.CriticRating ?? 0),
			(ItemSortBy.DateCreated, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.DateCreated ?? new DateTimeOffset()),
			(ItemSortBy.DatePlayed, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.UserData?.LastPlayedDate ?? new DateTimeOffset()),
			(ItemSortBy.PlayCount, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.UserData?.PlayCount ?? 0),
			(ItemSortBy.PremiereDate, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.PremiereDate ?? new DateTimeOffset()),
			(ItemSortBy.Runtime, SortOrder.Ascending) => SortExpressionComparer<BaseItemDto>.Ascending(x => x.RunTimeTicks ?? 0),

			(ItemSortBy.Name, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.Name ?? ""),
			(ItemSortBy.CommunityRating, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.CommunityRating ?? 0),
			(ItemSortBy.CriticRating, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.CriticRating ?? 0),
			(ItemSortBy.DateCreated, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.DateCreated ?? new DateTimeOffset()),
			(ItemSortBy.DatePlayed, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.UserData?.LastPlayedDate ?? new DateTimeOffset()),
			(ItemSortBy.PlayCount, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.UserData?.PlayCount ?? 0),
			(ItemSortBy.PremiereDate, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.PremiereDate ?? new DateTimeOffset()),
			(ItemSortBy.Runtime, SortOrder.Descending) => SortExpressionComparer<BaseItemDto>.Descending(x => x.RunTimeTicks ?? 0),

			_ => SortExpressionComparer<BaseItemDto>.Ascending(x => x.Name ?? ""),
		};
	}

	public ReadOnlyObservableCollection<BaseItemDto> Items => _items;

	[ObservableProperty]
	public partial int NumberOfPages { get; set; }

	[ObservableProperty]
	public partial int SelectedPage { get; set; }

	[ObservableProperty]
	public partial ItemSortBy SortBy { get; set; } = ItemSortBy.Name;

	[ObservableProperty]
	public partial SortOrder Order { get; set; } = SortOrder.Ascending;

	[RelayCommand]
	public void UpdateSortBy(ItemSortBy sortBy) => SortBy = sortBy;

	[RelayCommand]
	public void UpdateSortOrder(SortOrder order) => Order = order;

	public int PageSize { get; } = 100;

	public Task OnNavigatedFrom() => Task.CompletedTask;

	public async Task OnNavigatedTo(object parameter)
	{
		if (parameter is not BaseItemDto libraryDto)
		{
			return;
		}

		var result = await _jellyfinClient.GetItems(libraryDto);

		if(result is null or { Items : null })
		{
			return;
		}

		NumberOfPages = (int)Math.Ceiling((double)result.Items.Count / PageSize);
		_itemsCache.AddOrUpdate(result.Items);
	}

here are some tests

		[Fact]
		public async Task WhenSinglePage()
		{
			var vm = new LibraryViewModel(new TestClient(), 100);
			await vm.OnNavigatedTo(new BaseItemDto());

			Assert.Equal("A", vm.Items[0].Name);

			vm.UpdateSortBy(ItemSortBy.CriticRating);
			vm.UpdateSortOrder(SortOrder.Descending);

			Assert.Equal("C", vm.Items[0].Name);
		}

		[Fact]
		public async Task WhenMultiplePage()
		{
			var vm = new LibraryViewModel(new TestClient(), 2);
			await vm.OnNavigatedTo(new BaseItemDto());

			Assert.Equal("A", vm.Items[0].Name);

			vm.UpdateSortBy(ItemSortBy.CriticRating);
			vm.UpdateSortOrder(SortOrder.Descending);

			Assert.Equal("C", vm.Items[0].Name);
		}
		
	        public class TestClient : IJellyfinClient
	        {
		      public async Task<BaseItemDtoQueryResult?> GetItems(BaseItemDto parent)
		      {
			      await Task.Delay(1);
			      return new BaseItemDtoQueryResult
			      {
				      Items =
				      [
					      new(){ Name = "A", CriticRating = 5, Id = Guid.NewGuid() },
					      new(){ Name = "B", CriticRating = 6, Id = Guid.NewGuid() },
					      new(){ Name = "C", CriticRating = 7, Id = Guid.NewGuid() },
				      ]
			      };
		      }
	         }

[JakenVeina]

Alright, I see the issue.

There's a little bit of a trick going on with how sorting works, within a keyed changeset. That is, there's no such thing as sorting within a keyed changeset. The .SortAndPage() operator works by using extended changesets, with allow the operator to attach contextual information to each changeset, in addition to the list of changes. When a re-sort operation happens, what .SortAndPage() does is publish a list of the normal keyed changes (I.E. adds/removes/replaces) and attaches the current state of paging within the context, which most-significantly includes the sorting comparer. So, downstream operators are expected to detect when the comparer changes, and re-sort the collection, locally, which is (in theory) more efficient than trying to build a list of individual move or swap changes.

The .SortAndPage() operator then has a lovely little optimization at the very end of its processing chain that filters out empty changesets. Since we're dealing with a re-sort on a single page, there are no adds or removes, so the changeset ends up empty, even though the comparer changed, and the changeset model itself isn't capable of flagging that some other significant change occurred, so whoever added that optimization didn't catch that just checking the change count isn't enough to determine if the changeset is superfluous.

Should be a very easy fix, plus some time spent searching to see if it applies to other operators. Expect a patch fix in the somewhat-near future.

[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.