Client reset w/recovery #4711

kneth · 2022-07-08T11:59:14Z

What, How & Why?

This closes #4135

☑️ ToDos

📝 Changelog entry
📝 Compatibility label is updated or copied from previous entry
🚦 Tests
🔀 Executed flexible sync tests locally if modifying flexible sync
📦 Updated internal package version in consuming package.jsons (if updating internal packages)
📱 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~~

kraenhansen · 2022-10-13T13:53:10Z

types/index.d.ts

+    enum ClientResetDidRecover {
+      Yes = 'yes',
+      No = 'no'
+    }


Why is this not just a simple boolean?

It would allow for latter adding "perhaps - not entirely sure" 🤷

undefined would be a great way to express that in that theoretical future.

I suggest making this a boolean for now if we're not certain that new values will be added soon.

Alternatively I'd suggest using values that are closer to semantics:

Suggested change

enum ClientResetDidRecover {

Yes = 'yes',

No = 'no'

}

enum ClientResetOutcome {

Recovered = 'recovered',

Discarded = 'discarded'

}

Another alternative (implemented in other SDKs) is to provide a third callback (similar to the callback for manual mode) which is called when recovery is not possible - or when an error occurs.

integration-tests/realm-apps/with-db-flx/functions/triggerClientReset/config.json

integration-tests/realm-apps/with-db/functions/triggerClientReset/config.json

kraenhansen · 2022-10-13T13:55:59Z

src/js_sync.hpp

+        arguments[0] = create_object<T, RealmClass<T>>(m_ctx, new SharedRealm(before_realm));
+        arguments[1] = create_object<T, RealmClass<T>>(m_ctx, new SharedRealm(after_realm));
+        if (!m_ignore_recover) {
+            arguments[2] = Value<T>::from_string(m_ctx, did_recover ? "yes" : "no");


I'd suggest this should simply be a boolean in the user-facing API.

An enum is specified in the technical design and will align better with other SDKs

kraenhansen · 2022-10-13T13:58:36Z

types/index.d.ts

    type ClientResetAfterCallback = (localRealm: Realm, remoteRealm: Realm) => void;
-    interface ClientResetConfiguration<ClientResetModeT = ClientResetMode> {
-        mode: ClientResetModeT;
-        clientResetBefore?: ClientResetBeforeCallback;
-        clientResetAfter?: ClientResetAfterCallback;
+    type ClientResetAfterRecoveryCallback = (localRealm: Realm, remoteRealm: Realm, didRecover: ClientResetDidRecover) => void;


Could we merge the ClientResetAfterCallback and ClientResetAfterRecoveryCallback if we provided false as default value for didRecover?

didRecover is something the sync client sets. Moreover, I want to express that didRecover do not make sense in all cases.

I think it should simply call this with false if it couldn't happen. Users providing this callback can simply choose not to read that third argument. I believe the complexity added doesn't outweigh the benefits here.

kraenhansen · 2022-10-19T16:38:01Z

tests/js/session-tests.js

@@ -1330,7 +1213,7 @@ module.exports = {

        13)  synced, unencrypted Realm -> synced, unencrypted Realm
        14)  synced, unencrypted Realm -> synced, encrypted Realm
-        15)  synced, encrypted Realm -> synced, unencrypted Realm
+        15)  synced, encrypted Realm -> synced, unencrypted Relam


Suggested change

15) synced, encrypted Realm -> synced, unencrypted Relam

15) synced, encrypted Realm -> synced, unencrypted Realm

kraenhansen · 2022-10-19T16:38:27Z

tests/js/session-tests.js

@@ -361,7 +361,7 @@ module.exports = {
    const credentials = Realm.Credentials.anonymous();
    return app.logIn(credentials).then((user) => {
      return new Promise((resolve, _reject) => {
-        const config = getSyncConfiguration(user, partition);
+        let config = getSyncConfiguration(user, partition);


Seem like an unneeded change to me.

kraenhansen

I think this API makes more sense 👍

kraenhansen · 2022-10-24T08:29:01Z

lib/extensions.js

      Manual: "manual",
-      DiscardLocal: "discardLocal",
+      DiscardUnsyncedChanges: "discardUnsyncedChanges",
+      RecoverUnsyncedChanges: "recoverUnsyncedChanges",
+      RecoverOrDiscardUnsyncedChanges: "recoverOrDiscardUnsyncedChanges",


This is missing: DiscardLocal = 'discardLocal'

Co-authored-by: Kræn Hansen <[email protected]>

* Add new client reset modes * Expand manual mode Co-authored-by: Kræn Hansen <[email protected]>

takameyer · 2022-11-14T11:33:40Z

src/js_sync.hpp

@@ -438,13 +612,6 @@ void SessionClass<T>::get_config(ContextType ctx, ObjectType object, ReturnValue
                                 Value::from_string(ctx, String::from_bson(partition_value_bson)));
        }

-        auto conf = session->config();


@kneth What was the reasoning of removing error from the returned sync configuration?

cla-bot bot added the cla: yes label Jul 8, 2022

kneth mentioned this pull request Jul 8, 2022

[iOS] Client reset due to breaking schema change crashes until reinstall #4699

Closed

kneth self-assigned this Jul 8, 2022

kneth force-pushed the feature/client-reset-recovery branch from 2a4c222 to a5371ce Compare August 4, 2022 15:32

kneth force-pushed the feature/client-reset-recovery branch from 4a73a46 to dff7204 Compare September 22, 2022 11:48

kneth force-pushed the feature/client-reset-recovery branch from 1b0b9f1 to 2b93d65 Compare October 1, 2022 08:01

kneth force-pushed the feature/client-reset-recovery branch from 7219b78 to e753044 Compare October 12, 2022 07:11

kneth marked this pull request as ready for review October 13, 2022 13:36

kraenhansen reviewed Oct 13, 2022

View reviewed changes

integration-tests/realm-apps/with-db-flx/functions/triggerClientReset/config.json Outdated Show resolved Hide resolved

kraenhansen reviewed Oct 13, 2022

View reviewed changes

integration-tests/realm-apps/with-db/functions/triggerClientReset/config.json Outdated Show resolved Hide resolved

kraenhansen reviewed Oct 13, 2022

View reviewed changes

kneth changed the base branch from master to v10 October 18, 2022 12:03

kraenhansen reviewed Oct 19, 2022

View reviewed changes

kraenhansen approved these changes Oct 24, 2022

View reviewed changes

kraenhansen reviewed Oct 24, 2022

View reviewed changes

kneth force-pushed the feature/client-reset-recovery branch from e845294 to ee75117 Compare October 25, 2022 13:39

kneth and others added 11 commits October 26, 2022 08:42

Update BAAS

8ccbb84

Add new client reset modes

6d4a82f

basic support for client reset w/recovery

9cdf764

Migrate existing client reset tests

cbf471a

adding first tests

eed600a

Update API

5da73bd

Expand manual mode

bfe3eba

Refactor sync error handlers. Add TS types

b1be3e7

Add tests for manual client reset

026e52c

remove space

8acdab9

Add triggerClientReset

e5b1b9d

kneth and others added 15 commits October 26, 2022 08:42

Server-side client reset

c07df56

TS defs for client reset

893c56b

Fix warning. Fix linting errors.

20720ae

Trigger client reset

f566c2c

wip

c839965

wip

87e0d7b

Also testing flexible sync

5212512

wip

5d23205

Fix test

b3f5189

disable flx tests

7208c54

Apply suggestions from code review

d0e09a2

Co-authored-by: Kræn Hansen <[email protected]>

Rename to UnsyncedChanges

f3c514f

Update API + docs + tests + TS defs

98b632f

C++ code formatting

65b1bc1

Fix session test

e28658a

kneth force-pushed the feature/client-reset-recovery branch from 6c6603e to e28658a Compare October 26, 2022 06:42

kneth added 5 commits October 26, 2022 10:44

Increase timeout for client reset tests to an insane large value

28fb013

Suppress destruction on node/electron for callbacks

579733a

Increase timeout

b40e3c5

wip

a110a89

lint

2ca2df1

kneth merged commit a54c953 into v10 Oct 31, 2022

kneth deleted the feature/client-reset-recovery branch October 31, 2022 10:02

kneth added a commit that referenced this pull request Nov 1, 2022

Client reset w/recovery (#4711)

5c8cf79

* Add new client reset modes * Expand manual mode Co-authored-by: Kræn Hansen <[email protected]>

kneth added a commit that referenced this pull request Nov 1, 2022

Client reset w/recovery (#4711) (#5047)

ad79d5f

* Add new client reset modes * Expand manual mode Co-authored-by: Kræn Hansen <[email protected]>

takameyer reviewed Nov 14, 2022

View reviewed changes

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

Client reset w/recovery #4711

Client reset w/recovery #4711

kneth commented Jul 8, 2022 •

edited

Loading

kraenhansen Oct 13, 2022

bmunkholm Oct 13, 2022

kraenhansen Oct 19, 2022

kneth Oct 20, 2022

kraenhansen Oct 13, 2022 •

edited

Loading

kneth Oct 14, 2022

kraenhansen Oct 13, 2022

kneth Oct 14, 2022

kraenhansen Oct 19, 2022 •

edited

Loading

kraenhansen Oct 19, 2022

kraenhansen Oct 19, 2022

kraenhansen left a comment

kraenhansen Oct 24, 2022

takameyer Nov 14, 2022

	15) synced, encrypted Realm -> synced, unencrypted Relam
	15) synced, encrypted Realm -> synced, unencrypted Realm

Client reset w/recovery #4711

Client reset w/recovery #4711

Conversation

kneth commented Jul 8, 2022 • edited Loading

What, How & Why?

☑️ ToDos

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen Oct 13, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen Oct 19, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kraenhansen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kneth commented Jul 8, 2022 •

edited

Loading

kraenhansen Oct 13, 2022 •

edited

Loading

kraenhansen Oct 19, 2022 •

edited

Loading