Expose subscriptions more directly #6231

cmelchior · 2018-10-12T18:24:28Z

Expose Subscriptions more directly, so it is easier to create, find and remove them. This mirrors the API's in realm/realm-js#2060 but exposes the Subscription class more directly as it makes it easier to query, list and manipulate the subscriptions.

Added new Subscription model class that represents __ResultSets objects.
Added Realm.getSubscriptions(), Realm.getSubscriptions(String pattern), Realm.getSubscription(String name).
• Added RealmQuery.subscribe() that creates the subscription directly (requires a write transaction).
If SyncConfiguration.waitForInitialRemoteData() is set and SyncConfiguration.initialData() creates subscriptions, the Realm isn't opened until the subscriptions have downloaded data.

TODO:

Changelog
Merge Add support for creating a subscription object immediately realm-object-store#701 and switch back to using OS master
Fix Don't create __ResultSets realm-object-store#698
Add tests for initialData and getInstanceAsync
Added tests for Subscription class.

… small fixes

…nces are minimized.

cmelchior · 2018-10-18T12:27:02Z

Ready for review. The unit test failing is unrelated to the changes here(it is however pretty consistent, so I'll be looking into it)

kneth

👍

I have added a number of minor comments (mostly documentation)

CHANGELOG.md

realm/realm-library/src/main/java/io/realm/RealmCache.java

realm/realm-library/src/main/java/io/realm/sync/Subscription.java

kneth · 2018-10-19T08:08:36Z

realm/realm-library/src/main/java/io/realm/sync/Subscription.java

+ * device. If an object is not covered by any subscription it will be removed from the device,
+ * but not the server.
+ * <p>
+ * Subscriptions are Realm objects, so deleting them by e.g. calling {@link RealmObject#deleteFromRealm()},


"by e.g." -> "e.g. by"

realm/realm-library/src/main/java/io/realm/sync/Subscription.java

realm/realm-library/src/objectServer/java/io/realm/internal/SyncObjectServerFacade.java

nhachicha · 2018-10-22T12:20:52Z

realm/realm-library/src/main/java/io/realm/RealmCache.java

+     * @param realmFileExistedOnDisk {@code true} if the file existed on disk before trying to open the Realm.
+     */
+    private static void synchronizeInitialSubscriptionsIfNeeded(Realm realm, boolean realmFileExistedOnDisk) {
+        if (!realmFileExistedOnDisk) {


why not do this check in https://github.com/realm/realm-java/pull/6231/files#diff-79e6ca3beee0a83dd76772285b606694R351 realmFileExistedOnDisk is not needed in this function, maybe change the name to synchronizeInitialSubscriptions as well

realmFileExistedOnDisk is set before creating anything at the entry of the function, but I agree the name isn't very descriptive at this point in time. I changed the name to realmFileIsBeingCreated and reversed the boolean logic in the places required.

The check is here because initialData is only ever called for typed Realms, not DynamicRealm.

nhachicha · 2018-10-22T12:25:02Z

realm/realm-library/src/main/java/io/realm/RealmCache.java

@@ -340,6 +339,17 @@ private static RealmCache getCache(String realmPath, boolean createIfNotExist) {
            if (realmClass == Realm.class) {
                // RealmMigrationNeededException might be thrown here.
                realm = Realm.createInstance(this);
+
+                // If `waitForInitialRemoteData` data is set, we also want to ensure that all subscriptions
+                // are fully ready before proceeding. The `initialData` block is only executed for


~~How can be there any subscription(s) before opening the Realm?~~ Ok via initialData

realm/realm-library/src/main/java/io/realm/RealmQuery.java

realm/realm-library/src/main/java/io/realm/RealmResults.java

realm/realm-library/src/main/java/io/realm/sync/Subscription.java

realm/realm-library/src/objectServer/java/io/realm/internal/SyncObjectServerFacade.java

realm/realm-library/src/syncIntegrationTest/java/io/realm/objectserver/QueryBasedSyncTests.java

# Conflicts: # CHANGELOG.md

nhachicha

Nice PR, this should solve a bunch of issues with the subscription/initial state. 👍 if CI is happy

…alm-java into cm/better-subscriptions

tgoyne · 2018-10-24T17:07:27Z

Maybe the query API is sufficiently different to begin with that it isn't a big deal, but it's potentially confusing to add .subscribe() and .unsubscribe() methods with different semantics from what the the methods with those names do in all of the other SDKs.

Nothing else really stands out to me.

bmunkholm · 2018-10-24T17:56:16Z

CHANGELOG.md

@@ -29,7 +49,7 @@
 * APIs are backwards compatible with all previous release of realm-java in the 5.x.y series.

 ### Internal
-* None
+* Updated to Object Store commit: 362b886628b3aefc5b7a0bc32293d794dc1d4ad5


Does that solve any issues in Java?

bmunkholm · 2018-10-24T18:52:57Z

Regarding the comment from Thomas:

Maybe the query API is sufficiently different to begin with that it isn't a big deal, but it's potentially confusing to add .subscribe() and .unsubscribe() methods with different semantics from what the methods with those names do in all of the other SDKs.

I think it's a problem if we make naming the same but semantics different. We should avoid that if at all possible.

cmelchior · 2018-10-24T19:38:44Z

I think it's a problem if we make naming the same but semantics different. We should avoid that if at all possible.

Yes, we should. But in this case, it isn't possible.

It is virtually impossible to make these API's similar as the way queries are performed and run are so very different between Java and Cocoa/JS and changing that would be a massive, massive change for all existing users.

The name subscribe() encapsulates the meaning of actually creating the subscription on both platforms, so even though the semantics are different, the end result is more or less the same.

… starting the next one.

cmelchior added 2 commits September 13, 2018 09:30

WIP

2a7a93c

Add Javadoc. Remove problematic getSubscription on RealmResults.

730a750

cmelchior added the T-Enhancement label Oct 12, 2018

cmelchior self-assigned this Oct 12, 2018

realm-ci added the Thinking Robot label Oct 12, 2018

realm-probot bot removed the Thinking Robot label Oct 12, 2018

cmelchior added 12 commits October 12, 2018 20:51

WIP

0e1a41c

Merge branch 'master' into cm/better-subscriptions

02b1b67

Add subscription capabilities on RealmQuery

87413b8

Add Subscription to query based module

be99baa

Add Realm tests. Fix a few bugs plus and docs.

9d9eeae

Add RealmQuery tests

76066ab

Use ObjectStore implementation as much as possible.

a3c1e94

Added changelog

43767a9

Refactor Realm initialization to support loading subscriptions. Other…

99a24f4

… small fixes

Use correct version of Object Store

16cf0ab

Work-around for adding __ResultSets twice

41b50bc

Fix Findbugs warnings

381d989

Zhuinden mentioned this pull request Oct 15, 2018

Cannot query synced realm using PagedLiveResults Zhuinden/realm-monarchy#18

Closed

cmelchior added 3 commits October 15, 2018 20:49

Update Object Store. Revert RealmCache changes so opening Realm insta…

0c41625

…nces are minimized.

Use OS master

fe37952

Add missing field from schema

025e3a2

cmelchior requested review from nhachicha and kneth October 18, 2018 12:26

kneth reviewed Oct 19, 2018

View reviewed changes

PR feedback

322e1b4

nhachicha reviewed Oct 22, 2018

View reviewed changes

cmelchior added 2 commits October 23, 2018 12:11

Merge branch 'master' into cm/better-subscriptions

5a1a4b5

# Conflicts: # CHANGELOG.md

PR feedback

5f0b95e

nhachicha approved these changes Oct 23, 2018

View reviewed changes

cmelchior added 2 commits October 23, 2018 16:13

Attempt to fix flaky test

e4fa7d2

Merge branch 'cm/better-subscriptions' of https://github.com/realm/re…

b2ad370

…alm-java into cm/better-subscriptions

bmunkholm requested a review from tgoyne October 24, 2018 10:05

bmunkholm reviewed Oct 24, 2018

View reviewed changes

Fix unstable tests by moving reset into tearing down tests instead of…

dc1ff5b

… starting the next one.

cmelchior force-pushed the cm/better-subscriptions branch from 3114cd3 to dc1ff5b Compare October 26, 2018 16:49

cmelchior merged commit 1c71cac into master Oct 26, 2018

cmelchior deleted the cm/better-subscriptions branch October 26, 2018 17:14

github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Expose subscriptions more directly #6231

Expose subscriptions more directly #6231

cmelchior commented Oct 12, 2018 •

edited

Loading

cmelchior commented Oct 18, 2018

kneth left a comment

kneth Oct 19, 2018

nhachicha Oct 22, 2018

cmelchior Oct 23, 2018

nhachicha Oct 22, 2018 •

edited

Loading

nhachicha left a comment

tgoyne commented Oct 24, 2018

bmunkholm Oct 24, 2018

bmunkholm commented Oct 24, 2018

cmelchior commented Oct 24, 2018

Expose subscriptions more directly #6231

Expose subscriptions more directly #6231

Conversation

cmelchior commented Oct 12, 2018 • edited Loading

cmelchior commented Oct 18, 2018

kneth left a comment

Choose a reason for hiding this comment

kneth Oct 19, 2018

Choose a reason for hiding this comment

nhachicha Oct 22, 2018

Choose a reason for hiding this comment

cmelchior Oct 23, 2018

Choose a reason for hiding this comment

nhachicha Oct 22, 2018 • edited Loading

Choose a reason for hiding this comment

nhachicha left a comment

Choose a reason for hiding this comment

tgoyne commented Oct 24, 2018

bmunkholm Oct 24, 2018

Choose a reason for hiding this comment

bmunkholm commented Oct 24, 2018

cmelchior commented Oct 24, 2018

cmelchior commented Oct 12, 2018 •

edited

Loading

nhachicha Oct 22, 2018 •

edited

Loading