Using IncrementalLoadingSource in AdvancedCollectionView Causes NullReferenceException when adding a SortDescription

[k-g-nolan]

Describe the bug

When adding a SortDescription to an AdvancedCollectionView that has an IncrementalLoadingCollection as its source, a null reference exception is generated:

System.InvalidOperationException
  HResult=0x80131509
  Message=Failed to compare two elements in the array.
  Source=System.Private.CoreLib
  StackTrace:
   at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource, Exception e)
   at System.Collections.Generic.ArraySortHelper`1.Sort(Span`1 keys, IComparer`1 comparer)
   at System.Array.Sort[T](T[] array, Int32 index, Int32 length, IComparer`1 comparer)
   at System.Collections.Generic.List`1.Sort(Int32 index, Int32 count, IComparer`1 comparer)
   at CommunityToolkit.WinUI.Collections.AdvancedCollectionView.HandleSortChanged()
   at CommunityToolkit.WinUI.Collections.AdvancedCollectionView.SortDescriptions_CollectionChanged(Object sender, NotifyCollectionChangedEventArgs e)
   at System.Collections.ObjectModel.ObservableCollection`1.OnCollectionChanged(NotifyCollectionChangedEventArgs e)
   at ESx.ViewModels.PlcLogViewModel.<>c__DisplayClass41_0.<.ctor>b__0(Task _) in C:\Users\knolan\source\repos\ESx\ESx\ViewModels\PlcLogViewModel.cs:line 189
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)

  This exception was originally thrown at this call stack:
    CommunityToolkit.WinUI.Collections.AdvancedCollectionView.System.Collections.Generic.IComparer<object>.Compare(object, object)
    System.Collections.Generic.ArraySortHelper<T>.SwapIfGreater(System.Span<T>, System.Comparison<T>, int, int)
    System.Collections.Generic.ArraySortHelper<T>.PickPivotAndPartition(System.Span<T>, System.Comparison<T>)
    System.Collections.Generic.ArraySortHelper<T>.IntroSort(System.Span<T>, int, System.Comparison<T>)
    System.Collections.Generic.ArraySortHelper<T>.IntrospectiveSort(System.Span<T>, System.Comparison<T>)
    System.Collections.Generic.ArraySortHelper<T>.Sort(System.Span<T>, System.Collections.Generic.IComparer<T>)

Inner Exception 1:
NullReferenceException: Object reference not set to an instance of an object.

Steps to reproduce

WinUI CommunityToolkit 8.1 running on Windows 11 Pro 22631.4890, compiled for .net8

1. Create an `IncrementalLoadingCollection` for a user-defined class.
2. Call `RefreshAsync()` to load items into the collection.
3. Follow that call by creating an `AdvancedCollectionView`, using the `IncrementalLoadingCollection` as its source.
4. Add a sort description to the `AdvancedCollectionView`. This will generate a null reference exception.

Expected behavior

AdvancedCollectionView should sort the items in the list based on the given SortDescription.

Screenshots

Code Platform

• UWP
• WinAppSDK / WinUI 3
• Web Assembly (WASM)
• Android
• iOS
• MacOS
• Linux / GTK

Windows Build Number

• Windows 10 1809 (Build 17763)
• Windows 10 1903 (Build 18362)
• Windows 10 1909 (Build 18363)
• Windows 10 2004 (Build 19041)
• Windows 10 20H2 (Build 19042)
• Windows 10 21H1 (Build 19043)
• Windows 10 21H2 (Build 19044)
• Windows 10 22H2 (Build 19045)
• Windows 11 21H2 (Build 22000)
• Other (specify)

Other Windows Build number

Windows 11 23H2 (Build 22631.4890)

App minimum and target SDK version

• Windows 10, version 1809 (Build 17763)
• Windows 10, version 1903 (Build 18362)
• Windows 10, version 1909 (Build 18363)
• Windows 10, version 2004 (Build 19041)
• Windows 10, version 2104 (Build 20348)
• Windows 11, version 22H2 (Build 22000)
• Other (specify)

Other SDK version

Windows 11, build 22621

Visual Studio Version

2022

Visual Studio Build Number

17.12.3

Device form factor

Desktop

Additional context

Here is the code in question:

// "Log" is a user-defined class with DateTime property "Timestamp"
var collection = new IncrementalLoadingCollection<LogSource, Log>(new LogSource(_logManager), PAGE_SIZE);

_ = collection.RefreshAsync()
          .ContinueWith(_ => {
              CurrentLogEntries = new AdvancedCollectionView(collection, true);
              CurrentLogEntries.SortDescriptions.Add(new SortDescription(nameof(Log.Timestamp), SortDirection.Descending));  // error happens here.
          }, TaskContinuationOptions.ExecuteSynchronously);  // Work to initialize the ACV is done on the UI thread.

I checked the code for the AdvancedCollectionView, and I believe this is happening because of how the ACV handles generic types. It assumes the object type is whatever the first generic argument is:

// In AdvancedCollectionView.cs:


#pragma warning disable CA1033 // Interface methods should be callable by child types
    int IComparer<object>.Compare(object x, object y)
#pragma warning restore CA1033 // Interface methods should be callable by child types
    {
        if (!_sortProperties.Any())
        {
            var listType = _source?.GetType();
            Type type;

            if (listType != null && listType.IsGenericType)
            {
                /* For IncrementalLoadingCollection, the first generic argument is NOT the type of the objects in the list*/
                type = listType.GetGenericArguments()[0];  // type is some IIncrementalLoadingSource
            }
            else
            {
                type = x.GetType();
            }

            foreach (var sd in _sortDescriptions)
            {
                if (!string.IsNullOrEmpty(sd.PropertyName))
                {
                    _sortProperties[sd.PropertyName] = type.GetProperty(sd.PropertyName);  // GetProperty() likely returns null here because type is not the correct type.
                }
            }
        }

        foreach (var sd in _sortDescriptions)
        {
            object cx, cy;

            if (string.IsNullOrEmpty(sd.PropertyName))
            {
                cx = x;
                cy = y;
            }
            else
            {
                var pi = _sortProperties[sd.PropertyName];  // pi is null here

                cx = pi.GetValue(x!);  // Likely NullReferenceException thrown here
                cy = pi.GetValue(y!);
            }

            var cmp = sd.Comparer.Compare(cx, cy);

            if (cmp != 0)
            {
                return sd.Direction == SortDirection.Ascending ? +cmp : -cmp;
            }
        }

        return 0;
    }

I was able to work around it by creating a wrapper for IncrementalLoadingCollection that switches the type arguments:

private sealed class IncrementalLoadingWrapper<T, TSource> : IncrementalLoadingCollection<TSource, T> where TSource : IIncrementalSource<T>
{
    public IncrementalLoadingWrapper(TSource source, int pageSize) : base(source, pageSize) {}
}

Help us help you

Yes, but only if others can assist.

[k-g-nolan]

Created PR #643 to fix this issue.