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

Flexible sync #4220

Merged
merged 8 commits into from
Jan 24, 2022
Merged

Flexible sync #4220

merged 8 commits into from
Jan 24, 2022

Conversation

tomduncalf
Copy link
Contributor

@tomduncalf tomduncalf commented Jan 13, 2022

What, How & Why?

This PR implements flexible sync support.

Note: once this PR is merged, you will need to supply BAAS_AWS_ACCESS_KEY_ID and BAAS_AWS_SECRET_ACCESS_KEY env vars when starting the Docker image.

Example

// Open a Realm with flexible sync enabled
const realm = new Realm({
  schema: [PersonSchema, DogSchema],
  sync: { user, flexible: true }
});

// ------------------------------------------------

// Get the current set of subscriptions
const subs = realm.getSubscriptions();

console.log(subs)
// Subscriptions { }
console.log(subs.empty);
// true
console.log(subs.snapshot());
// []

// ------------------------------------------------

// Add a subscription - updates can only be performed inside an `update` callback,
// and mutating methods only exist on the `MutableSubscriptions` class
subs.update(mutableSubs => {
  console.log(mutableSubs);
  // MutableSubscriptions {}
  mutableSubs.add(realm.objects("Person"));
});

console.log(subs.empty);
// false
console.log(subs.snapshot());
// [Subscription]
console.log(subs.state);
// "pending" - indicates the server has not finished processing this 
// update so the data locally may be missing/incomplete

// ------------------------------------------------

// Wait for server synchronisation to be complete
await subs.waitForSynchronisation();

console.log(subs.state);
// "complete" - all initial data is synced and we are in steady state synchronisation. 
// Data we/others add/remove which matches the subscription will be synced.

// ------------------------------------------------

let filteredSub;

// Add several subscriptions in a batch - this is efficient for the server,
// as the client ORs the queries together before sending, resulting in a 
// single update on the server side (which is an expensive operation)
subs.update(mutableSubs => {
  // Add a filtered subscription, and keep a reference to it
  filteredSub = mutableSubs.add(realm.objects("Person").filtered("age > 20"));
  // Add a named subscriptions
  mutableSubs.add(realm.objects("Dog"), { name: "dogSubs" });
});

// ------------------------------------------------

// Find a subscription by name
console.log(subs.findByName("dogSubs"))
// Subscription

// Find a subscription by query and inspect some of its properties
const foundSub = subs.find(realm.objects("Person").filtered("age > 20"));
console.log(foundSub.queryString);
// "age > 20"
console.log(foundSub.objectType);
// "Person"
console.log(foundSub.name);
// null

// ------------------------------------------------

// Various ways of removing subscriptions 
subs.update(mutableSubs => {
  // Remove by query - returns true if a subscription was removed
  mutableSubs.remove(realm.objects("Person"));
  // Remove by name - returns true if a subscription was removed
  mutableSubs.removeByName("dogSubs");
  // Remove by reference - returns true if a subscription was removed
  mutableSubs.removeSubscription(filteredSub);
  // Remove by object type - returns count of removed subscriptions
  mutableSubs.removeByObjectType("Person");
  // Remove all subscriptions - returns count of removed subscriptions
  mutableSubs.removeAll();
});

☑️ ToDos

  • This shouldn't be merged until https://github.com/10gen/baas/pull/5414 is merged as apps cannot be imported without a hack in the app importer right now (removed for this PR: fbb6360)
  • Determine correct behaviour for a couple of xit'ed tests
  • Add more integration tests for complex scenarios, different data types etc. (maybe reuse mixed.ts tests for data types?)
  • 📝 Changelog entry
  • 📝 Compatibility label is updated or copied from previous entry
  • 🚦 Tests
  • 📱 Check the React Native/other sample apps work if necessary
  • 📝 Public documentation PR created or is not necessary
  • 💥 Breaking label has been applied or is not necessary

If this PR adds or changes public API's:

  • typescript definitions file is updated
  • jsdoc files updated
  • Chrome debug API is updated if API is available on React Native

@cla-bot cla-bot bot added the cla: yes label Jan 13, 2022
@tomduncalf tomduncalf closed this Jan 13, 2022
@tomduncalf tomduncalf reopened this Jan 13, 2022
@tomduncalf tomduncalf force-pushed the flexible-sync branch 3 times, most recently from b959218 to fbb6360 Compare January 17, 2022 16:44
@tomduncalf tomduncalf marked this pull request as ready for review January 17, 2022 16:46
docs/flexible-sync.js Outdated Show resolved Hide resolved
Copy link
Contributor

@takameyer takameyer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work! Just have a few things I picked out and a possible discussion over naming conventions.

CONTRIBUTING.md Outdated Show resolved Hide resolved
docs/flexible-sync.js Outdated Show resolved Hide resolved
docs/flexible-sync.js Show resolved Hide resolved
docs/flexible-sync.js Outdated Show resolved Hide resolved
docs/flexible-sync.js Outdated Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
CHANGELOG.md Outdated Show resolved Hide resolved
docs/flexible-sync.js Show resolved Hide resolved
src/js_realm.hpp Outdated Show resolved Hide resolved
src/js_realm.hpp Show resolved Hide resolved
src/js_subscriptions.hpp Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
src/js_subscriptions.hpp Outdated Show resolved Hide resolved
docs/flexible-sync.js Outdated Show resolved Hide resolved
docs/flexible-sync.js Outdated Show resolved Hide resolved
docs/flexible-sync.js Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
@tomduncalf tomduncalf force-pushed the flexible-sync branch 2 times, most recently from c8023d4 to ab81762 Compare January 20, 2022 14:30
.vscode/launch.json Outdated Show resolved Hide resolved
Copy link
Member

@kraenhansen kraenhansen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm submitting what I've managed to review today. There's still code that I haven't read yet.

Comment on lines 504 to 573
it("throws an error if no subscriptions have been created", async function (this: RealmContext) {
expect(this.realm.subscriptions.waitForSynchronization()).to.be.rejectedWith(
"`waitForSynchronisation` cannot be called before creating a subscription set using `update`",
);
});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering what the right behaviour is here.

Considering the case that Nikola mentioned where the app is killed before the flexible sync session has stabilised. In this case the user might add a call to realm.subscriptions.waitForSynchronization to the startup of their app? If that's true, it might be better to just resolve the promise if there's nothing to wait for? This way the user doesn't have to check if there's a subscription before awaiting this.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah it's an interesting point. I'll bring this up for discussion and see if we can unify on an approach to it. I think if this is the desired behaviour, ideally core would behave like that rather than having SDK specific logic

integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
integration-tests/tests/src/tests/sync/flexible.ts Outdated Show resolved Hide resolved
@tomduncalf tomduncalf merged commit 62edb6b into master Jan 24, 2022
@tomduncalf tomduncalf deleted the flexible-sync branch January 24, 2022 11:24
@tomduncalf tomduncalf mentioned this pull request Feb 1, 2022
9 tasks
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants