From c2de29ab0cde82b56d6d620071c0a6c916db0348 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Mon, 18 Dec 2017 14:46:26 -0800 Subject: [PATCH 1/3] Fix an issue where `Realm.open` would complain about the Realm already being open with a different schema version In order to correctly open read-only synchronized Realms, `Realm.open` would open the Realm without specifying a schema or schema version, wait for any remote changes to be downloaded (if appropriate), and then re-open the Realm with the specified schema and schema version. This would lead to an exception about the Realm being open with a different schema version if the Realm had previously been opened with a different schema version, due to the way `RealmCoordinator` caches information about the schema of open Realms. We address this by making two changes: 1. `Realm.open` for non-synchronized Realms no longer goes through `_waitForDownload`. This means the dance described above where the Realm is opened twice is not used for local Realms. 2. `_waitForDownload` no longer keeps the `Realm` alive until after its callback has returned. It instead keeps the `SyncSession` alive. This is sufficient to avoid the connection being torn down and having to reconnect when `_waitForDownload`'s callback later opens the Realm with the correct schema and schema version, while also allowing for `RealmCoordinator`'s cached information to be cleared when the schemaless Realm is closed prior to the Realm being reopened. In addition, tests have been added that reproduced the problem in both a local and sync context. --- CHANGELOG.md | 1 + lib/extensions.js | 21 +++++-- src/js_realm.hpp | 113 +++++++++++++++++++------------------- tests/js/realm-tests.js | 15 +++++ tests/js/session-tests.js | 44 +++++++++++++++ 5 files changed, 131 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2f4c6e77..acdd4c22 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ ### Bug fixes * [Object Server] Fixed a bug where long reconnection happens when a proxy in front of the sync worker returns one of those. +* Fix a bug where `Realm.open` could unexpectedly raise an "Realm at path ... already opened with different schema version" error. ### Internal * Updated to Realm Sync 2.1.10 (see "Bug fixes"). diff --git a/lib/extensions.js b/lib/extensions.js index 467e3347..f1c04695 100644 --- a/lib/extensions.js +++ b/lib/extensions.js @@ -62,16 +62,28 @@ module.exports = function(realmConstructor) { //Add async open API Object.defineProperties(realmConstructor, getOwnPropertyDescriptors({ open(config) { + // For local Realms we open the Realm and return it in a resolved Promise. + if (!("sync" in config)) { + let promise = Promise.resolve(new realmConstructor(config)); + promise.progress = (callback) => { }; + return promise; + } + + // For synced Realms we open the Realm without specifying the schema and then wait until + // the Realm has finished its initial sync with the server. We then reopen it with the correct + // schema. This avoids writing the schema to a potentially read-only Realm file, which would + // result in sync rejecting the writes. `_waitForDownload` ensures that the session is kept + // alive until our callback has returned, which prevents it from being torn down and recreated + // when we close the schemaless Realm and open it with the correct schema. let syncSession; let promise = new Promise((resolve, reject) => { let realm = new realmConstructor(waitForDownloadConfig(config)); realm._waitForDownload( - (session) => { - syncSession = session; - }, + (session) => { syncSession = session; }, (error) => { + realm.close(); if (error) { - setTimeout(() => { reject(error); }, 1); + setTimeout(() => { reject(error); }, 1); } else { try { @@ -91,7 +103,6 @@ module.exports = function(realmConstructor) { return promise; }; - return promise; }, diff --git a/src/js_realm.hpp b/src/js_realm.hpp index 3bd59f3a..5a434e64 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -245,9 +245,9 @@ public: {"close", wrap}, {"compact", wrap}, {"deleteModel", wrap}, - {"_waitForDownload", wrap}, {"_objectForObjectId", wrap}, #if REALM_ENABLE_SYNC + {"_waitForDownload", wrap}, {"_subscribeToObjects", wrap}, #endif }; @@ -720,6 +720,7 @@ void RealmClass::get_sync_session(ContextType ctx, ObjectType object, ReturnV } #endif +#if REALM_ENABLE_SYNC template void RealmClass::wait_for_download_completion(ContextType ctx, ObjectType this_object, Arguments args, ReturnValue &return_value) { args.validate_maximum(2); @@ -730,67 +731,63 @@ void RealmClass::wait_for_download_completion(ContextType ctx, ObjectType thi session_callback = Value::validated_to_function(ctx, args[0]); } -#if REALM_ENABLE_SYNC auto realm = *get_internal>(this_object); - if (auto* sync_config = realm->config().sync_config.get()) { - Protected protected_callback(ctx, callback_function); - Protected protected_this(ctx, this_object); - Protected protected_ctx(Context::get_global_context(ctx)); - - EventLoopDispatcher wait_handler([=](std::error_code error_code) { - HANDLESCOPE - if (!error_code) { - //success - Function::callback(protected_ctx, protected_callback, typename T::Object(), 0, nullptr); - } - else { - //fail - ObjectType object = Object::create_empty(protected_ctx); - Object::set_property(protected_ctx, object, "message", Value::from_string(protected_ctx, error_code.message())); - Object::set_property(protected_ctx, object, "errorCode", Value::from_number(protected_ctx, error_code.value())); - - ValueType callback_arguments[1]; - callback_arguments[0] = object; - - Function::callback(protected_ctx, protected_callback, typename T::Object(), 1, callback_arguments); - } - - // We keep our Realm instance alive until the callback has had a chance to open its own instance. - // This allows it to share the sync session that our Realm opened. - if (realm) - realm->close(); - }); - - std::shared_ptr user = sync_config->user; - if (user && user->state() != SyncUser::State::Error) { - if (auto session = user->session_for_on_disk_path(realm->config().path)) { - if (!Value::is_null(ctx, session_callback)) { - FunctionType session_callback_func = Value::to_function(ctx, session_callback); - auto syncSession = create_object>(ctx, new WeakSession(session)); - ValueType callback_arguments[1]; - callback_arguments[0] = syncSession; - Function::callback(protected_ctx, session_callback_func, typename T::Object(), 1, callback_arguments); - } - - session->wait_for_download_completion(std::move(wait_handler)); - return; - } - } - - ObjectType object = Object::create_empty(protected_ctx); - Object::set_property(protected_ctx, object, "message", - Value::from_string(protected_ctx, "Cannot asynchronously open synced Realm because the associated session previously experienced a fatal error")); - Object::set_property(protected_ctx, object, "errorCode", Value::from_number(protected_ctx, 1)); - - ValueType callback_arguments[1]; - callback_arguments[0] = object; - Function::callback(protected_ctx, protected_callback, protected_this, 1, callback_arguments); - return; + auto* sync_config = realm->config().sync_config.get(); + if (!sync_config) { + throw std::logic_error("_waitForDownload can only be used on a synchronized Realm."); } -#endif - Function::callback(ctx, callback_function, this_object, 0, nullptr); + Protected protected_callback(ctx, callback_function); + Protected protected_this(ctx, this_object); + Protected protected_ctx(Context::get_global_context(ctx)); + + std::shared_ptr user = sync_config->user; + if (user && user->state() != SyncUser::State::Error) { + if (auto session = user->session_for_on_disk_path(realm->config().path)) { + if (!Value::is_null(ctx, session_callback)) { + FunctionType session_callback_func = Value::to_function(ctx, session_callback); + auto syncSession = create_object>(ctx, new WeakSession(session)); + ValueType callback_arguments[1]; + callback_arguments[0] = syncSession; + Function::callback(protected_ctx, session_callback_func, typename T::Object(), 1, callback_arguments); + } + + EventLoopDispatcher wait_handler([=](std::error_code error_code) { + HANDLESCOPE + if (!error_code) { + //success + Function::callback(protected_ctx, protected_callback, typename T::Object(), 0, nullptr); + } + else { + //fail + ObjectType object = Object::create_empty(protected_ctx); + Object::set_property(protected_ctx, object, "message", Value::from_string(protected_ctx, error_code.message())); + Object::set_property(protected_ctx, object, "errorCode", Value::from_number(protected_ctx, error_code.value())); + + ValueType callback_arguments[1]; + callback_arguments[0] = object; + + Function::callback(protected_ctx, protected_callback, typename T::Object(), 1, callback_arguments); + } + // Ensure that the session remains alive until the callback has had an opportunity to reopen the Realm + // with the appropriate schema. + (void)session; + }); + session->wait_for_download_completion(std::move(wait_handler)); + return; + } + } + + ObjectType object = Object::create_empty(protected_ctx); + Object::set_property(protected_ctx, object, "message", + Value::from_string(protected_ctx, "Cannot asynchronously open synced Realm because the associated session previously experienced a fatal error")); + Object::set_property(protected_ctx, object, "errorCode", Value::from_number(protected_ctx, 1)); + + ValueType callback_arguments[1]; + callback_arguments[0] = object; + Function::callback(protected_ctx, protected_callback, protected_this, 1, callback_arguments); } +#endif template void RealmClass::objects(ContextType ctx, ObjectType this_object, Arguments args, ReturnValue &return_value) { diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index c0c22346..61a07b66 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -207,6 +207,21 @@ module.exports = { TestCase.assertEqual(realm.readOnly, true); }, + testRealmOpen: function() { + let realm = new Realm({schema: [schemas.TestObject], schemaVersion: 1}); + realm.write(() => { + realm.create('TestObject', [1]) + }); + realm.close(); + + return Realm.open({schema: [schemas.TestObject], schemaVersion: 2}).then(realm => { + const objects = realm.objects('TestObject'); + TestCase.assertEqual(objects.length, 1); + TestCase.assertEqual(objects[0].doubleCol, 1.0); + realm.close(); + }); + }, + testDefaultPath: function() { const defaultPath = Realm.defaultPath; let defaultRealm = new Realm({schema: []}); diff --git a/tests/js/session-tests.js b/tests/js/session-tests.js index c7859074..25232d84 100644 --- a/tests/js/session-tests.js +++ b/tests/js/session-tests.js @@ -169,6 +169,50 @@ module.exports = { }); }, + testRealmOpenWithExistingLocalRealm() { + if (!isNodeProccess) { + return; + } + + const username = uuid(); + const realmName = uuid(); + const expectedObjectsCount = 3; + + let user, config; + return runOutOfProcess(__dirname + '/download-api-helper.js', username, realmName, REALM_MODULE_PATH) + .then(() => Realm.Sync.User.login('http://localhost:9080', username, 'password')) + .then(u => { + user = u; + const accessTokenRefreshed = this; + let successCounter = 0; + + config = { + sync: { user, url: `realm://localhost:9080/~/${realmName}` }, + schema: [{ name: 'Dog', properties: { name: 'string' } }], + schemaVersion: 1, + }; + + // Open the Realm with a schema version of 1, then immediately close it. + // This verifies that Realm.open doesn't hit issues when the schema version + // of an existing, local Realm is different than the one passed in the configuration. + let realm = new Realm(config); + realm.close(); + + config.schemaVersion = 2; + return Realm.open(config) + }).then(realm => { + let actualObjectsCount = realm.objects('Dog').length; + TestCase.assertEqual(actualObjectsCount, expectedObjectsCount, "Synced realm does not contain the expected objects count"); + + const session = realm.syncSession; + TestCase.assertInstanceOf(session, Realm.Sync.Session); + TestCase.assertEqual(session.user.identity, user.identity); + TestCase.assertEqual(session.config.url, config.sync.url); + TestCase.assertEqual(session.config.user.identity, config.sync.user.identity); + TestCase.assertEqual(session.state, 'active'); + }); + }, + testRealmOpenAsync() { if (!isNodeProccess) { return; From a4e2f31138bbe836b819ef05d05c2ce8291157e3 Mon Sep 17 00:00:00 2001 From: Mark Rowe Date: Mon, 18 Dec 2017 18:04:50 -0800 Subject: [PATCH 2/3] Fix a typo in the change log --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index acdd4c22..7a985ad9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,7 @@ ### Bug fixes * [Object Server] Fixed a bug where long reconnection happens when a proxy in front of the sync worker returns one of those. -* Fix a bug where `Realm.open` could unexpectedly raise an "Realm at path ... already opened with different schema version" error. +* Fix a bug where `Realm.open` could unexpectedly raise a "Realm at path ... already opened with different schema version" error. ### Internal * Updated to Realm Sync 2.1.10 (see "Bug fixes"). From db8c166bb522dfc418f6b1feaa7ca694e8fdb608 Mon Sep 17 00:00:00 2001 From: blagoev Date: Tue, 19 Dec 2017 11:17:12 +0200 Subject: [PATCH 3/3] Update CHANGELOG.md --- CHANGELOG.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a985ad9..bec9119a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,17 @@ +X.Y.Z Release notes +============================================================= +### Breaking changes +* None. + +### Enhancements +* None. + +### Bug fixes +* Fix a bug where `Realm.open` could unexpectedly raise a "Realm at path ... already opened with different schema version" error. + +### Internal +* None + 2.1.1 Release notes (2017-12-15) ============================================================= ### Breaking changes