RealmResults is not synced in global listener #2926
Conversation
| @@ -268,6 +267,12 @@ void notifyAllListeners() { | |||
| if (!realm.isClosed() && threadContainsAsyncEmptyRealmObject()) { | |||
| updateAsyncEmptyRealmObject(); | |||
| } | |||
| // It is very important to notify the global listeners at last. | |||
| AllTypes allTypes = realm.where(AllTypes.class).findFirst(); | ||
| assertNotNull(allTypes); | ||
| allTypes.deleteFromRealm(); | ||
| final RealmResults<AllTypes> results = realm.where(AllTypes.class).findAll(); |
There was a problem hiding this comment.
Perhaps just realm.where(AllTypes.class).count() instead?
Close #2408 RealmResults are synced when calling its listener, since we need to check the table version before calling the listener. So sync it just after advance read won't be an option - in that way, the result's listener won't be triggered. So we notify the global listeners as the last thing to do, at that point, result will be synced already. Also a test case is added to ensure the calling sequence of synced listeners.
| @Test | ||
| @RunTestInLooperThread | ||
| public void realmListener_realmResultShouldBeSynced() { | ||
| final AtomicBoolean changedOnce = new AtomicBoolean(false); |
There was a problem hiding this comment.
Since you are actually using multiple states. It would probably read more easily if you used an AtomicInteger instead and did switches on the number
| @@ -268,6 +267,12 @@ void notifyAllListeners() { | |||
| if (!realm.isClosed() && threadContainsAsyncEmptyRealmObject()) { | |||
| updateAsyncEmptyRealmObject(); | |||
| } | |||
| // It is very important to notify the global listeners last. | |||
| // We don't sync RealmResults in realmChanged, instead, they are synced in notifySyncRealmResultsCallbacks. | |||
| // This is because of we need to compare the table view version to decide if it changes. Thus, we cannot sync | |||
There was a problem hiding this comment.
This is because we need to compare the TableView version in order to decide if it changed.
|
Good catch 💯 . Only had minor comments 👍 |
|
I think we should not promise the order of they get called ever... User's code logic should never rely on it. Even we might have a consistent order from now on, but that could be changed in the future. We can doc something like the orders of calling those listeners are not guaranteed. So, please don't rely on it? or just don't doc it which user should know it is a "undocumented" behaviour. |
|
I would probably be fine with just calling it out in the changelog, because as you say. We never promised any ordering. |
| realm.addChangeListener(new RealmChangeListener<Realm>() { | ||
| @Override | ||
| public void onChange(Realm element) { | ||
| switch (changeCounter.getAndAdd(1)) { |
There was a problem hiding this comment.
minor: similar to getAndIncrement
|
👍 minor things |
Close #2408
RealmResults are synced when calling its listener, since we need to
check the table version before calling the listener. So sync it just
after advance read won't be an option - in that way, the result's
listener won't be triggered.
So we notify the global listeners as the last thing to do, at that
point, result will be synced already.
Also a test case is added to ensure the calling sequence of synced
listeners.