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

Add keypath filtering to notifications #3501

Merged
merged 60 commits into from
Feb 9, 2024
Merged
Show file tree
Hide file tree
Changes from 46 commits
Commits
Show all changes
60 commits
Select commit Hold shift + click to select a range
a6a1b0d
A comment before pausing this for now
papafe Nov 30, 2023
f1e1877
Merge branch 'main' into fp/keypath-filtering
papafe Jan 11, 2024
af61b27
Stub
papafe Jan 12, 2024
996e583
Added basic tests and keypath to extension mehtods
papafe Jan 15, 2024
c915e0c
Returned to initial implementation
papafe Jan 15, 2024
be5c6cb
Stubs
papafe Jan 15, 2024
836fcbf
Updated core to v13.25.1
papafe Jan 15, 2024
0898049
Updated changelog
papafe Jan 16, 2024
a40558a
Merge branch 'fp/update-core' into fp/keypath-filtering
papafe Jan 16, 2024
1e27a79
Stub
papafe Jan 16, 2024
2d985b4
Stub
papafe Jan 17, 2024
eaf5406
Merge branch 'main' into fp/keypath-filtering
papafe Jan 17, 2024
a5f3b04
Adding tests
papafe Jan 18, 2024
44939c7
Fixed unsubuscription
papafe Jan 18, 2024
a7157d8
Added new tests
papafe Jan 18, 2024
c0415a4
Small cleaning up
papafe Jan 18, 2024
9e0065c
Added tests
papafe Jan 19, 2024
d8eb427
Added lots of tests
papafe Jan 19, 2024
74b940a
Added more tests
papafe Jan 19, 2024
284d34a
Moved tests down
papafe Jan 19, 2024
c414305
Slight improvement to handlers
papafe Jan 19, 2024
d391248
Corrected order of operations
papafe Jan 19, 2024
c269bb3
Improved docs
papafe Jan 22, 2024
c930def
Added tests
papafe Jan 22, 2024
c9ac4d3
Fixes
papafe Jan 23, 2024
dd48576
Improved API
papafe Jan 23, 2024
f453127
Improved API and corrected tests
papafe Jan 23, 2024
a4a5b07
Fixing API
papafe Jan 24, 2024
2459a09
Removed test
papafe Jan 24, 2024
f1733da
Small corrections
papafe Jan 24, 2024
54fbe60
Stub
papafe Jan 24, 2024
a967957
First step internal API unification
papafe Jan 25, 2024
e88ac84
almost finished unification
papafe Jan 25, 2024
8313fa5
Improved API unification
papafe Jan 25, 2024
35257aa
Simplified API and added tests for collection construction [skip-ci]
papafe Jan 25, 2024
5e4ce66
Small corrections
papafe Jan 25, 2024
6a8ba0e
Simplified with the use of marshaled vector
papafe Jan 25, 2024
3dba3ce
Moved keypaths related to another file
papafe Jan 26, 2024
96edc6e
Small corrections [skip-ci]
papafe Jan 26, 2024
11448bd
Small corrections
papafe Jan 26, 2024
2f96851
Divided implementation
papafe Jan 26, 2024
e6931d9
Fixed API for collections
papafe Jan 26, 2024
2f3428d
Corrected tests
papafe Jan 26, 2024
74eadb2
Added missing tests
papafe Jan 26, 2024
7f466c0
Small fix
papafe Jan 26, 2024
d3fa6f8
Removed comment
papafe Jan 26, 2024
a912e75
Fixes following PR
papafe Jan 26, 2024
caf4735
Apply suggestions from code review
papafe Jan 26, 2024
949d2c4
Various PR comments fixes
papafe Jan 26, 2024
32978d9
Various corrections according to PR
papafe Jan 30, 2024
9b07657
Merge branch 'main' into fp/keypath-filtering
papafe Feb 6, 2024
461f725
Updated changelog and docs
papafe Feb 6, 2024
dc29014
Fixes
papafe Feb 6, 2024
99b7a22
Changed marshaling to go around .net framework limitation
papafe Feb 6, 2024
59232bf
Proposed delegate change (#3512)
nirinchev Feb 7, 2024
436a80d
Small fix
papafe Feb 7, 2024
9b8667b
Update CHANGELOG.md
papafe Feb 7, 2024
9ed3181
Update Realm/Realm/DatabaseTypes/RealmCollectionBase.cs
papafe Feb 7, 2024
5d16d4f
Merge branch 'main' into fp/keypath-filtering
papafe Feb 8, 2024
8409f36
Merge branch 'main' into fp/keypath-filtering
papafe Feb 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion Realm/Realm/DatabaseTypes/Accessors/ManagedAccessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,8 @@ public void UnsubscribeFromNotifications()
}

/// <inheritdoc/>
void INotifiable<NotifiableObjectHandleBase.CollectionChangeSet>.NotifyCallbacks(NotifiableObjectHandleBase.CollectionChangeSet? changes, bool shallow)
void INotifiable<NotifiableObjectHandleBase.CollectionChangeSet>.NotifyCallbacks(NotifiableObjectHandleBase.CollectionChangeSet? changes,
KeyPathsCollectionType type, IntPtr callback)
{
if (changes.HasValue)
{
Expand Down
5 changes: 3 additions & 2 deletions Realm/Realm/DatabaseTypes/INotifiable.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ internal interface INotifiable<TChangeset>
/// Method called when there are changes to report for that object.
/// </summary>
/// <param name="changes">The changes that occurred.</param>
/// <param name="shallow">Whether the changes are coming from a shallow notifier or not.</param>
void NotifyCallbacks(TChangeset? changes, bool shallow);
/// <param name="type">The type of the key paths collection related to the notification.</param>
/// <param name="callbackNative">The eventual callback to call for the notification (if type == Full) </param>
void NotifyCallbacks(TChangeset? changes, KeyPathsCollectionType type, IntPtr callbackNative = default);
nirinchev marked this conversation as resolved.
Show resolved Hide resolved
}

internal class NotificationToken<TCallback> : IDisposable
Expand Down
114 changes: 114 additions & 0 deletions Realm/Realm/DatabaseTypes/KeyPathCollection.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
////////////////////////////////////////////////////////////////////////////
//
// Copyright 2023 Realm Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License")
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

using System;
using System.Collections;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

namespace Realms;

//TODO Add docs for this and the following types

Check warning on line 27 in Realm/Realm/DatabaseTypes/KeyPathCollection.cs

View workflow job for this annotation

GitHub Actions / Verify TODOs

Realm/Realm/DatabaseTypes/KeyPathCollection.cs#L27

TODO entry doesn't have a link to Github issue or Jira ticket Add docs for this and the following types
public class KeyPathsCollection : IEnumerable<KeyPath>
{
private IEnumerable<KeyPath> _collection;

private static readonly KeyPathsCollection _shallow = new KeyPathsCollection(KeyPathsCollectionType.Shallow);
private static readonly KeyPathsCollection _default = new KeyPathsCollection(KeyPathsCollectionType.Default);
papafe marked this conversation as resolved.
Show resolved Hide resolved

internal KeyPathsCollectionType Type { get; set; }
papafe marked this conversation as resolved.
Show resolved Hide resolved

private KeyPathsCollection(KeyPathsCollectionType type, IEnumerable<KeyPath>? collection = null)
papafe marked this conversation as resolved.
Show resolved Hide resolved
{
Debug.Assert(type == KeyPathsCollectionType.Full == (collection?.Any() == true), "If collection isn't empty, then the type must be Full");

Type = type;
_collection = collection ?? Enumerable.Empty<KeyPath>();

VerifyKeyPaths();
}

internal IEnumerable<string> GetStrings() => _collection.Select(x => x.Path);

internal void VerifyKeyPaths()
{
foreach (var item in _collection)
{
if (string.IsNullOrWhiteSpace(item?.Path))
{
throw new ArgumentException("A key path cannot be null, empty, or consisting only of white spaces");
}
}
}

public static KeyPathsCollection Of(params KeyPath[] paths)
{
if (paths.Length == 0)
{
return new KeyPathsCollection(KeyPathsCollectionType.Shallow);
}

return new KeyPathsCollection(KeyPathsCollectionType.Full, paths);
}

public static KeyPathsCollection Shallow => _shallow;

public static KeyPathsCollection Default => _default;

public static implicit operator KeyPathsCollection(List<string> paths) =>
new(KeyPathsCollectionType.Full, paths.Select(path => (KeyPath)path));

public static implicit operator KeyPathsCollection(List<KeyPath> paths) => new(KeyPathsCollectionType.Full, paths);

public static implicit operator KeyPathsCollection(string[] paths) =>
new(KeyPathsCollectionType.Full, paths.Select(path => (KeyPath)path));

public static implicit operator KeyPathsCollection(KeyPath[] paths) => new(KeyPathsCollectionType.Full, paths);

/// <inheritdoc/>
public IEnumerator<KeyPath> GetEnumerator()
{
return _collection.GetEnumerator();
}

/// <inheritdoc/>
IEnumerator IEnumerable.GetEnumerator()
{
return GetEnumerator();
}
}

public class KeyPath
papafe marked this conversation as resolved.
Show resolved Hide resolved
{
internal string Path { get; private set; }
papafe marked this conversation as resolved.
Show resolved Hide resolved

private KeyPath(string path)
{
Path = path;
}

public static implicit operator KeyPath(string s) => new(s);
papafe marked this conversation as resolved.
Show resolved Hide resolved
}

internal enum KeyPathsCollectionType
{
Default,
Shallow,
Full
papafe marked this conversation as resolved.
Show resolved Hide resolved
}
99 changes: 64 additions & 35 deletions Realm/Realm/DatabaseTypes/RealmCollectionBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
using Realms.Exceptions;
using Realms.Helpers;
using Realms.Schema;
using static Realms.NotifiableObjectHandleBase;

namespace Realms
{
Expand Down Expand Up @@ -189,15 +190,41 @@ public IRealmCollection<T> Freeze()
return CreateCollection(frozenRealm, frozenHandle);
}

public IDisposable SubscribeForNotifications(NotificationCallbackDelegate<T> callback)
=> SubscribeForNotificationsImpl(callback, shallow: false);
public IDisposable SubscribeForNotifications(NotificationCallbackDelegate<T> callback, KeyPathsCollection? keyPathCollection = null)
papafe marked this conversation as resolved.
Show resolved Hide resolved
{
keyPathCollection ??= KeyPathsCollection.Default;

if (keyPathCollection.Type == KeyPathsCollectionType.Full && !ContainsRealmObjects())
{
throw new InvalidOperationException("Key paths can be used only with collections of Realm objects");
papafe marked this conversation as resolved.
Show resolved Hide resolved
}

return SubscribeForNotificationsImpl(callback, keyPathCollection);
}

internal IDisposable SubscribeForNotificationsImpl(NotificationCallbackDelegate<T> callback, bool shallow)
internal IDisposable SubscribeForNotificationsImpl(NotificationCallbackDelegate<T> callback, KeyPathsCollection keyPathsCollection)
{
Argument.NotNull(callback, nameof(callback));
_notificationCallbacks.Value.Add(callback, shallow);

return NotificationToken.Create(callback, c => UnsubscribeFromNotifications(c, shallow));
if (keyPathsCollection.Type == KeyPathsCollectionType.Full)
{
var managedResultsHandle = GCHandle.Alloc(this, GCHandleType.Weak);
var callbackHandle = GCHandle.Alloc(callback, GCHandleType.Weak);

var token = Handle.Value.AddNotificationCallback(GCHandle.ToIntPtr(managedResultsHandle), keyPathsCollection,
GCHandle.ToIntPtr(callbackHandle));

return NotificationToken.Create(callback, c => token.Dispose());
}

// For notifications with type Default or Shallow we cache the callbacks on the managed level, to avoid creating multiple notifications in core
papafe marked this conversation as resolved.
Show resolved Hide resolved
_notificationCallbacks.Value.Add(callback, keyPathsCollection);
return NotificationToken.Create(callback, c => UnsubscribeFromNotifications(c, keyPathsCollection.Type));
}

protected virtual bool ContainsRealmObjects()
{
return typeof(IRealmObjectBase).IsAssignableFrom(typeof(T));
}

protected abstract T GetValueAtIndex(int index);
Expand Down Expand Up @@ -257,9 +284,9 @@ protected static IEmbeddedObject EnsureUnmanagedEmbedded(in RealmValue value)
return result;
}

private void UnsubscribeFromNotifications(NotificationCallbackDelegate<T> callback, bool shallow)
private void UnsubscribeFromNotifications(NotificationCallbackDelegate<T> callback, KeyPathsCollectionType type)
{
_notificationCallbacks.Value.Remove(callback, shallow);
_notificationCallbacks.Value.Remove(callback, type);
}

#region INotifyCollectionChanged
Expand Down Expand Up @@ -396,18 +423,31 @@ private void UpdateCollectionChangedSubscriptionIfNecessary(bool isSubscribed)
{
if (isSubscribed)
{
SubscribeForNotificationsImpl(OnChange, shallow: true);
SubscribeForNotificationsImpl(OnChange, KeyPathsCollection.Shallow);
}
else
{
UnsubscribeFromNotifications(OnChange, shallow: true);
UnsubscribeFromNotifications(OnChange, KeyPathsCollectionType.Shallow);
}
}
}

#endregion INotifyCollectionChanged

void INotifiable<NotifiableObjectHandleBase.CollectionChangeSet>.NotifyCallbacks(NotifiableObjectHandleBase.CollectionChangeSet? changes, bool shallow)
void INotifiable<CollectionChangeSet>.NotifyCallbacks(CollectionChangeSet? changes, KeyPathsCollectionType type, IntPtr callbackNative)
{
if (type == KeyPathsCollectionType.Full
&& GCHandle.FromIntPtr(callbackNative).Target is NotificationCallbackDelegate<T> callback)
{
callback(this, GetFromNative(changes));
return;
}

var changeset = GetFromNative(changes);
_notificationCallbacks.Value.Notify(changeset, type);
}

private ChangeSet? GetFromNative(CollectionChangeSet? changes)
papafe marked this conversation as resolved.
Show resolved Hide resolved
{
ChangeSet? changeset = null;
if (changes != null)
Expand All @@ -422,7 +462,7 @@ private void UpdateCollectionChangedSubscriptionIfNecessary(bool isSubscribed)
cleared: actualChanges.Cleared);
}

_notificationCallbacks.Value.Notify(changeset, shallow);
return changeset;
}

public IEnumerator<T> GetEnumerator() => new Enumerator(this);
Expand Down Expand Up @@ -611,19 +651,17 @@ internal class NotificationCallbacks<T>
{
private readonly RealmCollectionBase<T> _parent;

private readonly Dictionary<KeyPath, (NotificationTokenHandle? Token, bool DeliveredInitialNotification, List<NotificationCallbackDelegate<T>> Callbacks)> _subscriptions = new();
private readonly Dictionary<KeyPathsCollectionType, (NotificationTokenHandle? Token, bool DeliveredInitialNotification, List<NotificationCallbackDelegate<T>> Callbacks)> _subscriptions = new();

public NotificationCallbacks(RealmCollectionBase<T> parent)
{
_parent = parent;
}

// Shallow here and everywhere else is a bit of a shortcut we're taking until we have proper keypath filtering.
// Once we do, we probably want this to be some keypath identifier that we can pass between managed and native.
public void Add(NotificationCallbackDelegate<T> callback, bool shallow)
public void Add(NotificationCallbackDelegate<T> callback, KeyPathsCollection keyPathsCollection)
{
var keyPath = shallow ? KeyPath.Empty : KeyPath.Full;
if (_subscriptions.TryGetValue(keyPath, out var subscription))
var kpcType = keyPathsCollection.Type;
if (_subscriptions.TryGetValue(kpcType, out var subscription))
{
if (subscription.DeliveredInitialNotification)
{
Expand All @@ -638,29 +676,28 @@ public void Add(NotificationCallbackDelegate<T> callback, bool shallow)
else
{
// If this is a new subscription, we store it in the backing dictionary, then we subscribe outside of transaction
_subscriptions[keyPath] = (null, false, new() { callback });
_subscriptions[kpcType] = (null, false, new() { callback });
_parent.Realm.ExecuteOutsideTransaction(() =>
{
// It's possible that we unsubscribed in the meantime, so only add a notification callback if we still have callbacks
if (_subscriptions.TryGetValue(keyPath, out var sub) && sub.Callbacks.Count > 0)
if (_subscriptions.TryGetValue(kpcType, out var sub) && sub.Callbacks.Count > 0)
{
var managedResultsHandle = GCHandle.Alloc(_parent, GCHandleType.Weak);
_subscriptions[keyPath] = (_parent.Handle.Value.AddNotificationCallback(GCHandle.ToIntPtr(managedResultsHandle), shallow), sub.DeliveredInitialNotification, sub.Callbacks);
_subscriptions[kpcType] = (_parent.Handle.Value.AddNotificationCallback(GCHandle.ToIntPtr(managedResultsHandle), keyPathsCollection), sub.DeliveredInitialNotification, sub.Callbacks);
}
});
}
}

public bool Remove(NotificationCallbackDelegate<T> callback, bool shallow)
public bool Remove(NotificationCallbackDelegate<T> callback, KeyPathsCollectionType kpcType)
{
var keyPath = shallow ? KeyPath.Empty : KeyPath.Full;
if (_subscriptions.TryGetValue(keyPath, out var subscription))
if (_subscriptions.TryGetValue(kpcType, out var subscription))
{
subscription.Callbacks.Remove(callback);
if (subscription.Callbacks.Count == 0)
{
subscription.Token?.Dispose();
_subscriptions.Remove(keyPath);
_subscriptions.Remove(kpcType);
}

return true;
Expand All @@ -669,14 +706,13 @@ public bool Remove(NotificationCallbackDelegate<T> callback, bool shallow)
return false;
}

public void Notify(ChangeSet? changes, bool shallow)
public void Notify(ChangeSet? changes, KeyPathsCollectionType kpcType)
{
var keyPath = shallow ? KeyPath.Empty : KeyPath.Full;
if (_subscriptions.TryGetValue(keyPath, out var subscription))
if (_subscriptions.TryGetValue(kpcType, out var subscription))
{
if (changes == null)
{
_subscriptions[keyPath] = (subscription.Token, true, subscription.Callbacks);
_subscriptions[kpcType] = (subscription.Token, true, subscription.Callbacks);
}

foreach (var callback in subscription.Callbacks.ToArray())
Expand All @@ -695,13 +731,6 @@ public void RemoveAll()
}
}

// Eventually this should be a proper class holding the keypaths
internal enum KeyPath
{
Full,
Empty
}

/// <summary>
/// Special invalid object that is used to avoid an exception in WPF
/// when deleting an element from a collection bound to UI (<see href="https://github.com/realm/realm-dotnet/issues/1903">#1903</see>).
Expand Down
15 changes: 11 additions & 4 deletions Realm/Realm/DatabaseTypes/RealmDictionary.cs
Original file line number Diff line number Diff line change
Expand Up @@ -243,15 +243,22 @@ private static void ValidateKey(string key)
}
}

protected override bool ContainsRealmObjects()
{
return typeof(IRealmObjectBase).IsAssignableFrom(typeof(TValue));
}

internal override RealmCollectionBase<KeyValuePair<string, TValue>> CreateCollection(Realm realm, CollectionHandleBase handle) => new RealmDictionary<TValue>(realm, (DictionaryHandle)handle, Metadata);

internal override CollectionHandleBase GetOrCreateHandle() => _dictionaryHandle;

protected override KeyValuePair<string, TValue> GetValueAtIndex(int index) => _dictionaryHandle.GetValueAtIndex<TValue>(index, Realm);

void INotifiable<DictionaryHandle.DictionaryChangeSet>.NotifyCallbacks(DictionaryHandle.DictionaryChangeSet? changes, bool shallow)
void INotifiable<DictionaryHandle.DictionaryChangeSet>.NotifyCallbacks(DictionaryHandle.DictionaryChangeSet? changes,
KeyPathsCollectionType type, IntPtr callback)
{
Debug.Assert(!shallow, "Shallow should always be false here as we don't expose a way to configure it.");
Debug.Assert(type == KeyPathsCollectionType.Default,
"Notifications should always be default here as we don't expose a way to configure it.");

DictionaryChangeSet? changeset = null;
if (changes != null)
Expand All @@ -267,9 +274,9 @@ private static void ValidateKey(string key)
_deliveredInitialKeyNotification = true;
}

foreach (var callback in _keyCallbacks.ToArray())
foreach (var keyCallback in _keyCallbacks.ToArray())
{
callback(this, changeset);
keyCallback(this, changeset);
}
}
}
Expand Down
Loading
Loading