`SubscribeForNotifications` initial event should probably not be coalesced

[peppy]

What happened?

I have been tracking an occasional bug where our subscription handling was dying on index-out-of-range exceptions. It turns out that in a heavy usage scenario with ongoing modifications, the callback coalesces multiple events:

Notifications are delivered via the standard event loop, and so can't be delivered while the event loop is blocked by other activity. When notifications can't be delivered instantly, multiple notifications may be coalesced into a single notification. This can include the notification with the initial collection.

Unfortunately this includes the initial notification.

This is a puzzling API decision – as a user are you supposed to always create a tracking boolean to know when the initial population arrives? It seems that if you don't do this, it would not be possible to use the subscription for displaying in a view correctly (as you can't tell the difference between an empty list or yet-to-be-initially-populated list).

We have been writing change handling like this:

        /// <summary>
        /// Track GUIDs of all sets in realm to allow handling deletions.
        /// </summary>
        private readonly List<Guid> realmBeatmapSets = new List<Guid>();

        private void beatmapSetsChanged(IRealmCollection<BeatmapSetInfo> sender, ChangeSet? changes)
        {
            if (changes == null)
            {
                // Initial population.
                realmBeatmapSets.Clear();
                realmBeatmapSets.AddRange(sender.Select(r => r.ID));
            }
            else
            {
                foreach (int i in changes.DeletedIndices.OrderDescending())
                    realmBeatmapSets.RemoveAt(i);

                foreach (int i in changes.InsertedIndices)
                    realmBeatmapSets.Insert(i, sender[i].ID);
            }
        }

but we'd instead need to do something like this for correct handling:

        /// <summary>
        /// Track GUIDs of all sets in realm to allow handling deletions.
        /// </summary>
        private readonly List<Guid> realmBeatmapSets = new List<Guid>();

        private bool hasPopulated;

        private void beatmapSetsChanged(IRealmCollection<BeatmapSetInfo> sender, ChangeSet? changes)
        {
            if (!hasPopulated)
            {
                // Initial population.
                realmBeatmapSets.Clear();
                realmBeatmapSets.AddRange(sender.Select(r => r.ID));

                hasPopulated = true;
            }

            if (changes != null)
            {
                foreach (int i in changes.DeletedIndices.OrderDescending())
                    realmBeatmapSets.RemoveAt(i);

                foreach (int i in changes.InsertedIndices)
                    realmBeatmapSets.Insert(i, sender[i].ID);
            }
        }

It feels a bit counterintuitive to have to retain an isPopulated tracking bool for these subscriptions.

Also of note, the xmldoc on NotificationCallbackDelegate implies the initial callback will always be null:

Version

12.2.0

What Atlas Services are you using?

Local Database only

What type of application is this?

Other

Client OS and version

macOS (but all)

[sync-by-unito[bot]]

➤ PM Bot commented:

Jira ticket: RNET-1165

[nirinchev]

Just to clarify, the behavior you're observing is that you get the initial notification that already has changes populated? I agree that's likely not ideal as it makes it difficult to determine whether you need to setup the initial UI or mutate the existing containers.

[peppy]

@nirinchev precisely, yes. we're now working around this in a very simple fashion, which looks to be working well.

[nirinchev]

Sorry for getting back slowly on this - this is indeed a bug in the .NET SDK and the workaround you have is valid. We'll need to implement something similar.