Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

INotifyCollectionChanged implementation not compatible with ListCollectionView in WPF #3489

Closed
djymdmannen opened this issue Dec 11, 2023 · 6 comments

Comments

@djymdmannen
Copy link

What happened?

I'm evaluating Realm and how we'd go about replacing SQLite as a database in our WPF application.

One thing that I've encountered is that the INotifyCollectionChanged implementation can lead to issues when bound to a control that uses ListCollectionView, such as ListView.

Since ListCollectionView does not support range actions, it's very easy to trigger: add or remove two adjacent objects in a single transaction and you'll get a NotSupportedException saying "Range actions are not supported.".

ListCollectionView throws the exception whenever it detects range actions for:

  • NotifyCollectionChangedAction.Add
  • NotifyCollectionChangedAction.Remove
  • NotifyCollectionChangedAction.Replace
  • NotifyCollectionChangedAction.Move

Repro steps

  • Bind an IQueryable<T> to ListView.
  • In one transaction, either add or remove two adjacent objects.
    • Whether this is done on a thread-pool thread or on the UI thread does not matter.
  • ListCollectionView throws an exception with a message saying "Range actions are not supported."

Version

11.6.1

What Atlas Services are you using?

Local Database only

What type of application is this?

WPF

Client OS and version

Windows 11 23H2 22631.2792

Code snippets

No response

Stacktrace of the exception/crash you're getting

System.NotSupportedException: Range actions are not supported.
   at System.Windows.Data.ListCollectionView.ValidateCollectionChangedEventArgs(NotifyCollectionChangedEventArgs e)
   at System.Windows.Data.ListCollectionView.ProcessCollectionChanged(NotifyCollectionChangedEventArgs args)
   at System.Windows.Data.CollectionView.OnCollectionChanged(Object sender, NotifyCollectionChangedEventArgs args)
   at Realms.RealmCollectionBase`1.OnChange(IRealmCollection`1 sender, ChangeSet change)
   at Realms.NotificationCallbacks`1.Notify(ChangeSet changes, Boolean shallow)
   at Realms.RealmCollectionBase`1.Realms.INotifiable<Realms.NotifiableObjectHandleBase.CollectionChangeSet>.NotifyCallbacks(Nullable`1 changes, Boolean shallow)
   at Realms.NotifiableObjectHandleBase.NotifyObjectChanged(IntPtr managedHandle, CollectionChangeSet* changes, Boolean shallow)
   at Realms.SynchronizationContextScheduler.scheduler_invoke_function(IntPtr function_ptr, Boolean can_execute)
   at Realms.SynchronizationContextScheduler.Scheduler.<Post>b__5_0(Object f_ptr)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.DispatcherOperation.InvokeImpl()
   at System.Windows.Threading.DispatcherOperation.InvokeInSecurityContext(Object state)
   at MS.Internal.CulturePreservingExecutionContext.CallbackWrapper(Object obj)
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
--- End of stack trace from previous location ---
   at System.Threading.ExecutionContext.RunInternal(ExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Threading.ExecutionContext.Run(ExecutionContext executionContext, ContextCallback callback, Object state)
   at MS.Internal.CulturePreservingExecutionContext.Run(CulturePreservingExecutionContext executionContext, ContextCallback callback, Object state)
   at System.Windows.Threading.DispatcherOperation.Invoke()
   at System.Windows.Threading.Dispatcher.ProcessQueue()
   at System.Windows.Threading.Dispatcher.WndProcHook(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndWrapper.WndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam, Boolean& handled)
   at MS.Win32.HwndSubclass.DispatcherCallbackOperation(Object o)
   at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
   at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)
   at System.Windows.Threading.Dispatcher.LegacyInvokeImpl(DispatcherPriority priority, TimeSpan timeout, Delegate method, Object args, Int32 numArgs)
   at MS.Win32.HwndSubclass.SubclassWndProc(IntPtr hwnd, Int32 msg, IntPtr wParam, IntPtr lParam)
   at MS.Win32.UnsafeNativeMethods.DispatchMessage(MSG& msg)
   at System.Windows.Threading.Dispatcher.PushFrameImpl(DispatcherFrame frame)
   at System.Windows.Threading.Dispatcher.PushFrame(DispatcherFrame frame)
   at System.Windows.Threading.Dispatcher.Run()
   at System.Windows.Application.RunDispatcher(Object ignore)
   at System.Windows.Application.RunInternal(Window window)
   at System.Windows.Application.Run()
   at RealmTestAppWPF.App.Main()

Relevant log output

No response

@papafe
Copy link
Contributor

papafe commented Dec 11, 2023

Hi @djymdmannen, thanks for your report.
This is not a feature that we are planning on adding, but you could create a wrapper around the collections you need to bind to the UI. Something like:

public class Wrapper<T> : IEnumerable<T>, INotifyCollectionChanged
{
    private IEnumerable<T> inner;

    public event NotifyCollectionChangedEventHandler? CollectionChanged;

    public Wrapper(IEnumerable<T> inner)
    {
        this.inner = inner;
        (this.inner as INotifyCollectionChanged)!.CollectionChanged += Inner_CollectionChanged;
    }

    private void Inner_CollectionChanged(object? sender, NotifyCollectionChangedEventArgs e)
    {
        // Break down the range actions here

        CollectionChanged?.Invoke(this, e);
    }

    public IEnumerator<T> GetEnumerator()
    {
        return inner.GetEnumerator();
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return inner.GetEnumerator();
    }
}

To make it easer to use you could even create an extension method:

public static class Extension
{
    public static IEnumerable<T> Wrap<T>(this IEnumerable<T> collection)
    {
        return new Wrapper<T>(collection);
    }
}

And then use the value returned by the method in the UI:

UICollection = realm.All<RealmObj>().Wrap();

This is something I've wrote down quickly, so there could be some errors, but I hope that you got the point.

I'm going to close this ticket, as we're not planning to work on this, but feel free to open a new one if you have other issues.

@papafe papafe closed this as completed Dec 11, 2023
@djymdmannen
Copy link
Author

@papafe, That's too bad.

I was looking at that exact same work-around, but then it hit me that the wrapper needs to be applied to collection type properties as well? I'm assuming they all inherit from RealmCollectionBase<T>.

An example:

public partial class RealmObj : IRealmObject
{
    public IList<OrderedObject> Items { get; } = null!;

    [Ignored]
    public IList<OrderedObject> WrappedItems { get; } // create and return wrapper for Items property
}

It's getting ugly.

Where can I read more about the decision to not support WPF? I tried searching but came up short.

@papafe
Copy link
Contributor

papafe commented Dec 13, 2023

@djymdmannen I can understand your frustration, it's a shame default collections in WPF don’t support range actions while INotifyCollectionChanged allows it, and we were actually not aware of this limitation of WPF.

To clarify, we closed this issue because we don’t feel comfortable adding the possibility of flattening the notifications in our SDK, as developers could incur in the risk of inconsistencies, especially if they modify the collection they are subscribed to in the notification callback. An alternative would be to raise NotifyCollectionChangedAction.Reset for all ranged collection changes, as this would keep the access to the collection consistent.

Nevertheless, I've been experimenting a little bit on alternative (and hopefully more elegant) solutions to this problem, and I've come up with something based on converters.
There still need to be a Wrapper class, but it doesn't need to be generic:

public class Wrapper: IEnumerable, INotifyCollectionChanged
{
    private IEnumerable inner;

    public event NotifyCollectionChangedEventHandler? CollectionChanged;

    public Wrapper(IEnumerable inner)
    {
        this.inner = inner;
        (this.inner as INotifyCollectionChanged)!.CollectionChanged += Inner_CollectionChanged;
    }

    private void Inner_CollectionChanged(object? sender, NotifyCollectionChangedEventArgs e)
    {
        CollectionChanged?.Invoke(this, e);
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
        return inner.GetEnumerator();
    }
}

Then define a converter:

public class WrapCollectionConverter : IValueConverter
{
    public object Convert(object value, Type targetType, object parameter, CultureInfo culture)
    {
        if (value is IEnumerable ie)
        {
            return new Wrapper(ie);
        }

		//Shouldn't reach here
        throw new Exception();
    }

    public object ConvertBack(object value, Type targetType, object parameter, CultureInfo culture)
    {
		//Shouldn't reach here
        throw new Exception();
    }
}

And finally use it in the xaml:

<Window x:Class="TestWPF.MainWindow"
        xmlns:local="clr-namespace:TestWPF"
        Title="MainWindow" Height="450" Width="800">
    <Window.Resources>
        <local:WrapCollectionConverter x:Key="WrapCollectionConverter"/>
    </Window.Resources>
    <Grid>
        <ListView ItemsSource="{Binding Persons, Converter={StaticResource WrapCollectionConverter}}">
            <ListView.View>
                <GridView>
                    <GridViewColumn Header="Column 1" DisplayMemberBinding="{Binding Name}" />
                </GridView>
            </ListView.View>
        </ListView>
    </Grid>
</Window>

This solution has the advantage of not needing to define additional properties in the model classes for collection properties. Besides, I have experimented with this a little, and it seems you also don't even need to flatten the collection yourself with this approach, as internally WPF uses EnumerableCollectionView instead of ListCollectionView. This seems to be due to the fact that Wrapper does not implement the IList interface.

I do not have much experience with WPF, so I could be missing some details here. What do you think of this approach, would it be reasonable for your use case?

@djymdmannen
Copy link
Author

@papafe, Thank you, the solution looks interesting. I'll give it a try and I'll get back to you.

@nirinchev nirinchev reopened this Dec 14, 2023
@djymdmannen
Copy link
Author

djymdmannen commented Dec 15, 2023

@papafe, I've done some testing and it sure looks like this solution works. It is so much cleaner than having to duplicate collection properties and implement custom code that flattens the range actions. And not only that, the problem is also handled in the appropriate location (the view). And as far as work arounds go, I think this is as good as it gets, and you should mention this work around in the documentation. Great work, much appreciated!

For future readers of this issue:

if you're developing a WPF application and you bind to collections in Realm objects, you should use the suggested solution. If you do not, exceptions will be thrown in situations such as when adding two items to the collection in one transaction.

@papafe
Copy link
Contributor

papafe commented Dec 15, 2023

@djymdmannen Glad to hear this is working for you!
I'll close the issue, feel free to open a new one if you encounter other problems

@papafe papafe closed this as completed Dec 15, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 11, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants