From 7c2a6de57aca1bf15d41b1236121ff5601d8d837 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 9 May 2019 14:36:25 -0700 Subject: [PATCH 01/18] Pass more immutable values immediately --- lib/browser/collections.js | 4 ++-- lib/browser/index.js | 23 ++++++++++------------- lib/browser/user.js | 3 --- src/js_realm.hpp | 11 ++++++----- src/rpc.cpp | 26 ++++++++++++++++++-------- 5 files changed, 36 insertions(+), 31 deletions(-) diff --git a/lib/browser/collections.js b/lib/browser/collections.js index f258af0153..473a28eeca 100644 --- a/lib/browser/collections.js +++ b/lib/browser/collections.js @@ -121,10 +121,10 @@ export function createCollection(prototype, realmId, info, _mutable) { get: getterForProperty('length'), }, 'type': { - get: getterForProperty('type'), + value: info.dataType, }, 'optional': { - get: getterForProperty('optional'), + value: info.optional, }, }); diff --git a/lib/browser/index.js b/lib/browser/index.js index 2b4cc0afbe..972c22c679 100644 --- a/lib/browser/index.js +++ b/lib/browser/index.js @@ -43,30 +43,27 @@ rpc.registerTypeConverter(objectTypes.SUBSCRIPTION, createSubscription); function createRealm(_, info) { let realm = Object.create(Realm.prototype); - - setupRealm(realm, info.id); + setupRealm(realm, info); return realm; } -function setupRealm(realm, realmId) { - realm[keys.id] = realmId; - realm[keys.realm] = realmId; +function setupRealm(realm, info) { + realm[keys.id] = info.id; + realm[keys.realm] = info.id; realm[keys.type] = objectTypes.REALM; [ 'empty', - 'path', - 'readOnly', - 'inMemory', 'schema', 'schemaVersion', - 'syncSession', 'isInTransaction', 'isClosed', - '_isPartialRealm', ].forEach((name) => { Object.defineProperty(realm, name, {get: util.getterForProperty(name)}); }); + for (let key in info.data) { + realm[key] = rpc.deserialize(info.id, info.data[key]); + } } function getObjectType(realm, type) { @@ -102,11 +99,11 @@ export default class Realm { } } - let realmId = rpc.createRealm(Array.from(arguments)); - setupRealm(this, realmId); + let info = rpc.createRealm(Array.from(arguments)); + setupRealm(this, info); // This will create mappings between the id, path, and potential constructors. - objects.registerConstructors(realmId, this.path, constructors); + objects.registerConstructors(info.id, this.path, constructors); } create(type, ...args) { diff --git a/lib/browser/user.js b/lib/browser/user.js index a05add6b38..5689feaa55 100644 --- a/lib/browser/user.js +++ b/lib/browser/user.js @@ -43,9 +43,6 @@ export default class User { Object.defineProperties(User.prototype, { token: { get: getterForProperty('token') }, - server: { get: getterForProperty('server') }, - identity: { get: getterForProperty('identity') }, - isAdminToken: { get: getterForProperty('isAdminToken') }, }); createMethods(User.prototype, objectTypes.USER, [ diff --git a/src/js_realm.hpp b/src/js_realm.hpp index 0745cc9542..abdcbe0d10 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -261,7 +261,7 @@ class RealmClass : public ClassDefinition> { // static methods static void constructor(ContextType, ObjectType, Arguments &); static SharedRealm create_shared_realm(ContextType, realm::Realm::Config, bool, ObjectDefaultsMap &&, ConstructorMap &&); - static bool get_realm_config(ContextType ctx, ObjectType this_object, size_t argc, const ValueType arguments[], realm::Realm::Config &, ObjectDefaultsMap &, ConstructorMap &); + static bool get_realm_config(ContextType ctx, size_t argc, const ValueType arguments[], realm::Realm::Config &, ObjectDefaultsMap &, ConstructorMap &); static void schema_version(ContextType, ObjectType, Arguments &, ReturnValue &); static void clear_test_state(ContextType, ObjectType, Arguments &, ReturnValue &); @@ -459,7 +459,7 @@ static inline void convert_outdated_datetime_columns(const SharedRealm &realm) { } template -bool RealmClass::get_realm_config(ContextType ctx, ObjectType this_object, size_t argc, const ValueType arguments[], realm::Realm::Config& config, ObjectDefaultsMap& defaults, ConstructorMap& constructors) { +bool RealmClass::get_realm_config(ContextType ctx, size_t argc, const ValueType arguments[], realm::Realm::Config& config, ObjectDefaultsMap& defaults, ConstructorMap& constructors) { bool schema_updated = false; if (argc > 1) { @@ -563,7 +563,7 @@ bool RealmClass::get_realm_config(ContextType ctx, ObjectType this_object, si Value::from_number(ctx, used_bytes) }; - ValueType should_compact = Function::callback(ctx, should_compact_on_launch_function, this_object, 2, arguments); + ValueType should_compact = Function::callback(ctx, should_compact_on_launch_function, {}, 2, arguments); return Value::to_boolean(ctx, should_compact); }; } @@ -628,10 +628,11 @@ bool RealmClass::get_realm_config(ContextType ctx, ObjectType this_object, si template void RealmClass::constructor(ContextType ctx, ObjectType this_object, Arguments& args) { + set_internal>(this_object, nullptr); realm::Realm::Config config; ObjectDefaultsMap defaults; ConstructorMap constructors; - bool schema_updated = get_realm_config(ctx, this_object, args.count, args.value, config, defaults, constructors); + bool schema_updated = get_realm_config(ctx, args.count, args.value, config, defaults, constructors); auto realm = create_shared_realm(ctx, config, schema_updated, std::move(defaults), std::move(constructors)); // Fix for datetime -> timestamp conversion @@ -852,7 +853,7 @@ void RealmClass::async_open_realm(ContextType ctx, ObjectType this_object, Ar Realm::Config config; ObjectDefaultsMap defaults; ConstructorMap constructors; - bool schema_updated = get_realm_config(ctx, this_object, args.count - 1, args.value, config, defaults, constructors); + bool schema_updated = get_realm_config(ctx, args.count - 1, args.value, config, defaults, constructors); if (!config.sync_config) { throw std::logic_error("_asyncOpen can only be used on a synchronized Realm."); diff --git a/src/rpc.cpp b/src/rpc.cpp index b13636ba09..2a1aa1ca02 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -211,8 +211,7 @@ RPCServer::RPCServer() { } JSObjectRef realm_object = jsc::Function::construct(m_context, realm_constructor, arg_count, arg_values); - RPCObjectID realm_id = store_object(realm_object); - return (json){{"result", realm_id}}; + return (json){{"result", serialize_json_value(realm_object)}}; }; m_requests["/create_user"] = [this](const json dict) { JSObjectRef realm_constructor = get_realm_constructor(); @@ -604,8 +603,8 @@ json RPCServer::serialize_json_value(JSValueRef js_value) { return { {"type", RealmObjectTypesList}, {"id", store_object(js_object)}, - {"size", list->size()}, - {"schema", get_type(*list)}, + {"dataType", string_for_property_type(list->get_type() & ~realm::PropertyType::Flags)}, + {"optional", is_nullable(list->get_type())}, }; } else if (jsc::Object::is_instance>(m_context, js_object)) { @@ -613,20 +612,31 @@ json RPCServer::serialize_json_value(JSValueRef js_value) { return { {"type", RealmObjectTypesResults}, {"id", store_object(js_object)}, - {"size", results->size()}, - {"schema", get_type(*results)}, + {"dataType", string_for_property_type(results->get_type() & ~realm::PropertyType::Flags)}, + {"optional", is_nullable(results->get_type())}, }; } else if (jsc::Object::is_instance>(m_context, js_object)) { + json realm_dict { + {"_isPartialRealm", serialize_json_value(jsc::Object::get_property(m_context, js_object, "_isPartialRealm"))}, + {"inMemory", serialize_json_value(jsc::Object::get_property(m_context, js_object, "inMemory"))}, + {"path", serialize_json_value(jsc::Object::get_property(m_context, js_object, "path"))}, + {"readOnly", serialize_json_value(jsc::Object::get_property(m_context, js_object, "readOnly"))}, + {"syncSession", serialize_json_value(jsc::Object::get_property(m_context, js_object, "syncSession"))}, + }; return { {"type", RealmObjectTypesRealm}, {"id", store_object(js_object)}, + {"data", realm_dict} }; } else if (jsc::Object::is_instance>(m_context, js_object)) { auto user = *jsc::Object::get_internal>(js_object); json user_dict { - {"isAdmin", user->is_admin()} + {"identity", user->identity()}, + {"isAdmin", user->is_admin()}, + {"isAdminToken", user->token_type() == SyncUser::TokenType::Admin}, + {"server", user->server_url()}, }; return { {"type", RealmObjectTypesUser}, @@ -637,7 +647,7 @@ json RPCServer::serialize_json_value(JSValueRef js_value) { else if (jsc::Object::is_instance>(m_context, js_object)) { json session_dict { {"user", serialize_json_value(jsc::Object::get_property(m_context, js_object, "user"))}, - {"config", serialize_json_value(jsc::Object::get_property(m_context, js_object, "config"))} + {"config", serialize_json_value(jsc::Object::get_property(m_context, js_object, "config"))}, }; return { {"type", RealmObjectTypesSession}, From 73bf968f8a6a5c2b6efa40b7f3b8df142cd1d73c Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 9 May 2019 16:04:28 -0700 Subject: [PATCH 02/18] Add exponential backoff to callback_poll to reduce spam while idle --- lib/browser/rpc.js | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/lib/browser/rpc.js b/lib/browser/rpc.js index 2b595a4c5e..e2ff3e8cd7 100644 --- a/lib/browser/rpc.js +++ b/lib/browser/rpc.js @@ -231,6 +231,7 @@ function makeRequest(url, data) { } let pollTimeoutId; +let pollTimeout = 10; //returns an object from rpc serialized json value function deserialize_json_value(value) { @@ -260,6 +261,17 @@ function sendRequest(command, data, host = sessionHost) { let url = 'http://' + host + '/' + command; let response = makeRequest(url, data); + let callback = response && response.callback; + + // Reset the callback poll interval to 10ms every time we either hit a + // callback or call any other method, and double it each time we poll + // for callbacks and get nothing until it's over a second. + if (callback || command !== 'callbacks_poll') { + pollTimeout = 10; + } + else if (pollTimeout < 1000) { + pollTimeout *= 2; + } if (!response || response.error) { let error = response && response.error; @@ -315,6 +327,6 @@ function sendRequest(command, data, host = sessionHost) { return response.result; } finally { - pollTimeoutId = setTimeout(() => sendRequest('callbacks_poll'), 100); + pollTimeoutId = setTimeout(() => sendRequest('callbacks_poll'), pollTimeout); } } From fee71805142faed9ba980fbc884be142895d4cd6 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Fri, 10 May 2019 10:28:49 -0700 Subject: [PATCH 03/18] Expose the beforenotify event to JS --- src/js_realm.hpp | 145 +++++++++++++++++++++------------------- tests/js/realm-tests.js | 2 +- 2 files changed, 78 insertions(+), 69 deletions(-) diff --git a/src/js_realm.hpp b/src/js_realm.hpp index abdcbe0d10..62bbc6d165 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -67,7 +67,21 @@ class RealmClass; template class RealmDelegate : public BindingContext { - public: +private: + void did_change(std::vector const&, std::vector const&, bool) override { + notify(m_notifications, "change"); + } + + void schema_did_change(realm::Schema const& schema) override { + ObjectType schema_object = Schema::object_for_schema(m_context, schema); + notify(m_schema_notifications, "schema", schema_object); + } + + void before_notify() override { + notify(m_before_notify_notifications, "beforenotify"); + } + +public: using GlobalContextType = typename T::GlobalContext; using FunctionType = typename T::Function; using ObjectType = typename T::Object; @@ -77,14 +91,6 @@ class RealmDelegate : public BindingContext { using ObjectDefaultsMap = typename Schema::ObjectDefaultsMap; using ConstructorMap = typename Schema::ConstructorMap; - virtual void did_change(std::vector const& observers, std::vector const& invalidated, bool version_changed) { - notify("change"); - } - - virtual void schema_did_change(realm::Schema const& schema) { - schema_notify("schema", schema); - } - RealmDelegate(std::weak_ptr realm, GlobalContextType ctx) : m_context(ctx), m_realm(realm) {} ~RealmDelegate() { @@ -93,24 +99,15 @@ class RealmDelegate : public BindingContext { m_constructors.clear(); m_notifications.clear(); m_schema_notifications.clear(); + m_before_notify_notifications.clear(); } void add_notification(FunctionType notification) { - for (auto &handler : m_notifications) { - if (handler == notification) { - return; - } - } - m_notifications.emplace_back(m_context, notification); + add(m_notifications, notification); } void remove_notification(FunctionType notification) { - for (auto iter = m_notifications.begin(); iter != m_notifications.end(); ++iter) { - if (*iter == notification) { - m_notifications.erase(iter); - return; - } - } + remove(m_notifications, notification); } void remove_all_notifications() { @@ -119,28 +116,31 @@ class RealmDelegate : public BindingContext { void add_schema_notification(FunctionType notification) { SharedRealm realm = m_realm.lock(); - realm->read_group(); // to get the schema change handler going - for (auto &handler : m_schema_notifications) { - if (handler == notification) { - return; - } - } - m_schema_notifications.emplace_back(m_context, notification); + // schema change notifications only happen if the Realm has a read transaction active + realm->read_group(); + add(m_schema_notifications, notification); } void remove_schema_notification(FunctionType notification) { - for (auto iter = m_schema_notifications.begin(); iter != m_schema_notifications.end(); ++iter) { - if (*iter == notification) { - m_schema_notifications.erase(iter); - return; - } - } + remove(m_schema_notifications, notification); } void remove_all_schema_notifications() { m_schema_notifications.clear(); } + void add_before_notify_notification(FunctionType notification) { + add(m_before_notify_notifications, notification); + } + + void remove_before_notify_notification(FunctionType notification) { + remove(m_before_notify_notifications, notification); + } + + void remove_all_before_notify_notification() { + m_before_notify_notifications.clear(); + } + ObjectDefaultsMap m_defaults; ConstructorMap m_constructors; @@ -148,40 +148,39 @@ class RealmDelegate : public BindingContext { Protected m_context; std::list> m_notifications; std::list> m_schema_notifications; + std::list> m_before_notify_notifications; std::weak_ptr m_realm; - void notify(const char *notification_name) { - HANDLESCOPE - - SharedRealm realm = m_realm.lock(); - if (!realm) { - throw std::runtime_error("Realm no longer exists"); + void add(std::list>& notifications, FunctionType fn) { + if (std::find(notifications.begin(), notifications.end(), fn) != notifications.end()) { + return; } + notifications.emplace_back(m_context, std::move(fn)); + } - ObjectType realm_object = create_object>(m_context, new SharedRealm(realm)); - ValueType arguments[] = {realm_object, Value::from_string(m_context, notification_name)}; - - std::list> notifications_copy(m_notifications); - for (auto &callback : notifications_copy) { - Function::callback(m_context, callback, realm_object, 2, arguments); - } + void remove(std::list>& notifications, FunctionType fn) { + // This doesn't just call remove() because that would create a new Protected + notifications.remove_if([&](auto& notification) { return notification == fn; }); } - void schema_notify(const char *notification_name, realm::Schema const& schema) { + // Note that this intentionally copies the `notifications` argument as we + // want to iterate over a copy in case the user adds/removes notifications + // from inside the handler + template + void notify(std::list> notifications, const char *name, Args&&... args) { HANDLESCOPE - SharedRealm realm = m_realm.lock(); + auto realm = m_realm.lock(); if (!realm) { throw std::runtime_error("Realm no longer exists"); } ObjectType realm_object = create_object>(m_context, new SharedRealm(realm)); - ObjectType schema_object = Schema::object_for_schema(m_context, schema); - ValueType arguments[] = {realm_object, Value::from_string(m_context, notification_name), schema_object}; + ValueType arguments[] = {realm_object, Value::from_string(m_context, name), args...}; + auto argc = std::distance(std::begin(arguments), std::end(arguments)); - std::list> notifications_copy(m_schema_notifications); - for (auto &callback : notifications_copy) { - Function::callback(m_context, callback, realm_object, 3, arguments); + for (auto &callback : notifications) { + Function::callback(m_context, callback, realm_object, argc, arguments); } } @@ -352,14 +351,6 @@ class RealmClass : public ClassDefinition> { } } - static std::string validated_notification_name(ContextType ctx, const ValueType &value) { - std::string name = Value::validated_to_string(ctx, value, "notification name"); - if (name == "change" || name == "schema") { - return name; - } - throw std::runtime_error("Only the 'change' and 'schema' notification names are supported."); - } - static const ObjectSchema& validated_object_schema_for_value(ContextType ctx, const SharedRealm &realm, const ValueType &value) { std::string object_type; @@ -1126,7 +1117,7 @@ template void RealmClass::add_listener(ContextType ctx, ObjectType this_object, Arguments &args, ReturnValue &return_value) { args.validate_maximum(2); - auto name = validated_notification_name(ctx, args[0]); + std::string name = Value::validated_to_string(ctx, args[0], "notification name"); auto callback = Value::validated_to_function(ctx, args[1]); SharedRealm realm = *get_internal>(this_object); @@ -1134,16 +1125,22 @@ void RealmClass::add_listener(ContextType ctx, ObjectType this_object, Argume if (name == "change") { get_delegate(realm.get())->add_notification(callback); } - else { + else if (name == "beforenotify") { + get_delegate(realm.get())->add_before_notify_notification(callback); + } + else if (name == "schema") { get_delegate(realm.get())->add_schema_notification(callback); } + else { + throw std::runtime_error(util::format("Unknown event name '%1': only 'change', 'schema' and 'beforenotify' are supported.", name)); + } } template void RealmClass::remove_listener(ContextType ctx, ObjectType this_object, Arguments &args, ReturnValue &return_value) { args.validate_maximum(2); - auto name = validated_notification_name(ctx, args[0]); + std::string name = Value::validated_to_string(ctx, args[0], "notification name"); auto callback = Value::validated_to_function(ctx, args[1]); SharedRealm realm = *get_internal>(this_object); @@ -1151,9 +1148,15 @@ void RealmClass::remove_listener(ContextType ctx, ObjectType this_object, Arg if (name == "change") { get_delegate(realm.get())->remove_notification(callback); } - else { + else if (name == "beforenotify") { + get_delegate(realm.get())->remove_before_notify_notification(callback); + } + else if (name == "schema") { get_delegate(realm.get())->remove_schema_notification(callback); } + else { + throw std::runtime_error(util::format("Unknown event name '%1': only 'change', 'schema' and 'beforenotify' are supported", name)); + } } template @@ -1161,7 +1164,7 @@ void RealmClass::remove_all_listeners(ContextType ctx, ObjectType this_object args.validate_maximum(1); std::string name = "change"; if (args.count) { - name = validated_notification_name(ctx, args[0]); + name = Value::validated_to_string(ctx, args[0], "notification name"); } SharedRealm realm = *get_internal>(this_object); @@ -1169,9 +1172,15 @@ void RealmClass::remove_all_listeners(ContextType ctx, ObjectType this_object if (name == "change") { get_delegate(realm.get())->remove_all_notifications(); } - else { + else if (name == "beforenotify") { + get_delegate(realm.get())->remove_all_before_notify_notification(); + } + else if (name == "schema") { get_delegate(realm.get())->remove_all_schema_notifications(); } + else { + throw std::runtime_error(util::format("Unknown event name '%1': only 'change', 'schema' and 'beforenotify' are supported", name)); + } } template diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 27e18f6895..41e967656d 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -1011,7 +1011,7 @@ module.exports = { TestCase.assertEqual(secondNotificationCount, 1); TestCase.assertThrowsContaining(() => realm.addListener('invalid', () => {}), - "Only the 'change' and 'schema' notification names are supported."); + "Unknown event name 'invalid': only 'change', 'schema' and 'beforenotify' are supported."); realm.addListener('change', () => { throw new Error('expected error message'); From c08cdc4738d0368eed72836e9c90970364d5e1ce Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Fri, 10 May 2019 13:47:10 -0700 Subject: [PATCH 04/18] Better handle calls to clearTestState() in the middle of processing a notification --- src/rpc.cpp | 11 ++++++++++- src/rpc.hpp | 1 + 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/src/rpc.cpp b/src/rpc.cpp index 2a1aa1ca02..ba19145632 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -404,6 +404,7 @@ RPCServer::RPCServer() { m_callback_ids.clear(); m_callbacks[0] = refresh_access_token; m_callback_ids[refresh_access_token] = 0; + ++m_reset_counter; JSGarbageCollect(m_context); js::clear_test_state(); @@ -457,7 +458,15 @@ JSValueRef RPCServer::run_callback(JSContextRef ctx, JSObjectRef function, JSObj {"callback_call_counter", counter} }); - while (!server->try_run_task() && future.wait_for(std::chrono::microseconds(100)) != std::future_status::ready); + uint64_t reset_counter = server->m_reset_counter; + while (!server->try_run_task() && + future.wait_for(std::chrono::microseconds(100)) != std::future_status::ready && + reset_counter == server->m_reset_counter); + + if (reset_counter != server->m_reset_counter) { + // clearTestState() was called while the callback was pending + return JSValueMakeUndefined(ctx); + } json results = future.get(); // The callback id should be identical! diff --git a/src/rpc.hpp b/src/rpc.hpp index 5713b163ea..c02521efaf 100644 --- a/src/rpc.hpp +++ b/src/rpc.hpp @@ -86,6 +86,7 @@ class RPCServer { RPCObjectID m_session_id; RPCWorker m_worker; u_int64_t m_callback_call_counter; + uint64_t m_reset_counter = 0; std::mutex m_pending_callbacks_mutex; std::map, std::promise> m_pending_callbacks; From dbc3010d2247785a4fceaeec230cdaeb8b460f15 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Fri, 10 May 2019 11:31:22 -0700 Subject: [PATCH 05/18] Add proper read isolation to the RPC wrapper --- lib/browser/index.js | 4 ++-- lib/browser/rpc.js | 24 +++++++++++++++++++++++- src/rpc.cpp | 8 ++++++++ tests/js/realm-tests.js | 7 ++----- 4 files changed, 35 insertions(+), 8 deletions(-) diff --git a/lib/browser/index.js b/lib/browser/index.js index 972c22c679..34031f8ba1 100644 --- a/lib/browser/index.js +++ b/lib/browser/index.js @@ -198,8 +198,8 @@ Object.defineProperties(Realm, { }, }, _asyncOpen: { - value: function() { - return rpc.callMethod(undefined, Realm[keys.id], '_asyncOpen', Array.from(arguments)); + value: function(config, callback) { + return rpc.asyncOpenRealm(Realm[keys.id], config, callback); }, }, }); diff --git a/lib/browser/rpc.js b/lib/browser/rpc.js index e2ff3e8cd7..a37fee311c 100644 --- a/lib/browser/rpc.js +++ b/lib/browser/rpc.js @@ -57,12 +57,34 @@ export function createSession(refreshAccessToken, host) { return sessionId; } +function beforeNotify(realm) { + // No body required: the mere act of invoking this callback requires + // waiting for an event loop cycle in the browser and so achieves the goal + // of not letting notify() run when it wouldn't in normal usage +} + export function createRealm(args) { if (args) { args = args.map((arg) => serialize(null, arg)); } - return sendRequest('create_realm', { arguments: args }); + return sendRequest('create_realm', { arguments: args, beforeNotify: serialize(null, beforeNotify) }); +} + +export function asyncOpenRealm(id, config, callback) { + sendRequest('call_method', { + id, + name: '_asyncOpen', + arguments: [ + serialize(null, config), + serialize(null, (realm, error) => { + if (realm) { + realm.addListener('beforenotify', beforeNotify); + } + callback(realm, error); + }) + ] + }); } export function createUser(args) { diff --git a/src/rpc.cpp b/src/rpc.cpp index ba19145632..be04755ee4 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -211,6 +211,14 @@ RPCServer::RPCServer() { } JSObjectRef realm_object = jsc::Function::construct(m_context, realm_constructor, arg_count, arg_values); + + JSObjectRef add_listener_method = (JSObjectRef)jsc::Object::get_property(m_context, realm_object, "addListener"); + JSValueRef listener_args[] = { + jsc::Value::from_string(m_context, "beforenotify"), + deserialize_json_value(dict["beforeNotify"]) + }; + jsc::Function::call(m_context, add_listener_method, realm_object, 2, listener_args); + return (json){{"result", serialize_json_value(realm_object)}}; }; m_requests["/create_user"] = [this](const json dict) { diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 41e967656d..62167f9d1b 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -1489,11 +1489,8 @@ module.exports = { }); let realm2 = new Realm({ schema: schema, _cache: false }); - if (!isChromeWorker) { - // Not updated until we return to the event loop and the autorefresh can happen - // When running in Chrome this can happen at any time due to the async RPC - TestCase.assertEqual(realm1.schema.length, 0); - } + // Not updated until we return to the event loop and the autorefresh can happen + TestCase.assertEqual(realm1.schema.length, 0); TestCase.assertEqual(realm2.schema.length, 1); // give some time to let advance_read to complete From 49db673274e7788258c1fde6d401bd0d0772a411 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Tue, 14 May 2019 13:33:50 -0700 Subject: [PATCH 06/18] Fix a race condition in RPC callback handling With bad timing a top-level callback (intended for /callbacks_poll) could be picked up by something waiting for a result from a callback, resulting in a deadlock. --- src/rpc.cpp | 13 ++++++++++++- src/rpc.hpp | 1 + 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/src/rpc.cpp b/src/rpc.cpp index be04755ee4..329205a56b 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -125,10 +125,19 @@ json RPCWorker::add_task(Fn&& fn) { void RPCWorker::invoke_callback(json callback) { m_tasks.push_back([=, callback = std::move(callback)]() mutable { - if (auto promise = m_promises.try_pop_back(0)) { + if (m_depth == 1) { + // The callback was invoked directly from the event loop. Push it + // onto the queue of callbacks to be processed by /callbacks_poll + m_callbacks.push_back(std::move(callback)); + } + else if (auto promise = m_promises.try_pop_back(0)) { + // The callback was invoked from within a call to something else, + // and there's someone waiting for its result. promise->set_value(std::move(callback)); } else { + // The callback was invoked from within a call to something else, + // but there's no one waiting for the result. Shouldn't be possible? m_callbacks.push_back(std::move(callback)); } }); @@ -153,7 +162,9 @@ bool RPCWorker::try_run_task() { // Use a 10 millisecond timeout to keep this thread unblocked. if (auto task = m_tasks.try_pop_back(10)) { + ++m_depth; (*task)(); + --m_depth; return m_stop; } return false; diff --git a/src/rpc.hpp b/src/rpc.hpp index c02521efaf..f8affc8198 100644 --- a/src/rpc.hpp +++ b/src/rpc.hpp @@ -56,6 +56,7 @@ class RPCWorker { private: bool m_stop = false; + int m_depth = 0; #if __APPLE__ std::thread m_thread; CFRunLoopRef m_loop; From dabf3af5d075482542e508e3eaa902815aaac514 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 9 May 2019 16:11:58 -0700 Subject: [PATCH 07/18] Add property value caching to the RPC wrapper --- lib/browser/collections.js | 42 +++--------------- lib/browser/index.js | 12 ++--- lib/browser/objects.js | 7 ++- lib/browser/rpc.js | 26 +++++++++-- lib/browser/session.js | 6 +-- lib/browser/subscription.js | 10 +++-- lib/browser/util.js | 74 ++++++++++++++++++++++++------- src/rpc.cpp | 88 +++++++++++++++++++++++++++++++++++-- 8 files changed, 190 insertions(+), 75 deletions(-) diff --git a/lib/browser/collections.js b/lib/browser/collections.js index 473a28eeca..f7ea357226 100644 --- a/lib/browser/collections.js +++ b/lib/browser/collections.js @@ -19,10 +19,8 @@ 'use strict'; import { keys } from './constants'; -import { getterForProperty } from './util'; -import { getProperty, setProperty } from './rpc'; - -let mutationListeners = {}; +import * as util from './util'; +import * as rpc from './rpc'; export default class Collection { constructor() { @@ -30,29 +28,6 @@ export default class Collection { } } -export function addMutationListener(realmId, callback) { - let listeners = mutationListeners[realmId] || (mutationListeners[realmId] = new Set()); - listeners.add(callback); -} - -export function removeMutationListener(realmId, callback) { - let listeners = mutationListeners[realmId]; - if (listeners) { - listeners.delete(callback); - } -} - -export function clearMutationListeners() { - mutationListeners = {}; -} - -export function fireMutationListeners(realmId) { - let listeners = mutationListeners[realmId]; - if (listeners) { - listeners.forEach((cb) => cb()); - } -} - function isIndex(propertyName) { return typeof propertyName === 'number' || (typeof propertyName === 'string' && /^-?\d+$/.test(propertyName)); } @@ -62,7 +37,7 @@ const mutable = Symbol('mutable'); const traps = { get(collection, property, receiver) { if (isIndex(property)) { - return getProperty(collection[keys.realm], collection[keys.id], property); + return util.getProperty(collection, property); } return Reflect.get(collection, property, collection); @@ -73,13 +48,8 @@ const traps = { return false; } - setProperty(collection[keys.realm], collection[keys.id], property, value); - - // If this isn't a primitive value, then it might create a new object in the Realm. - if (value && typeof value == 'object') { - fireMutationListeners(collection[keys.realm]); - } - + util.invalidateCache(collection[keys.realm]); + rpc.setProperty(collection[keys.realm], collection[keys.id], property, value); return true; } @@ -118,7 +88,7 @@ export function createCollection(prototype, realmId, info, _mutable) { Object.defineProperties(collection, { 'length': { - get: getterForProperty('length'), + get: util.getterForProperty('length'), }, 'type': { value: info.dataType, diff --git a/lib/browser/index.js b/lib/browser/index.js index 34031f8ba1..bb9932f7bd 100644 --- a/lib/browser/index.js +++ b/lib/browser/index.js @@ -20,7 +20,7 @@ import { NativeModules } from 'react-native'; import { keys, objectTypes } from './constants'; -import Collection, * as collections from './collections'; +import Collection from './collections'; import List, { createList } from './lists'; import Results, { createResults } from './results'; import RealmObject, * as objects from './objects'; @@ -49,7 +49,7 @@ function createRealm(_, info) { function setupRealm(realm, info) { realm[keys.id] = info.id; - realm[keys.realm] = info.id; + realm[keys.realm] = info.realmId; realm[keys.type] = objectTypes.REALM; [ @@ -103,7 +103,7 @@ export default class Realm { setupRealm(this, info); // This will create mappings between the id, path, and potential constructors. - objects.registerConstructors(info.id, this.path, constructors); + objects.registerConstructors(info.realmId, this.path, constructors); } create(type, ...args) { @@ -127,7 +127,6 @@ util.createMethods(Realm.prototype, objectTypes.REALM, [ 'addListener', 'removeListener', 'removeAllListeners', - 'close', 'privileges', 'writeCopyTo', '_waitForDownload', @@ -141,6 +140,7 @@ util.createMethods(Realm.prototype, objectTypes.REALM, [ 'deleteAll', 'write', 'compact', + 'close', 'beginTransaction', 'commitTransaction', 'cancelTransaction', @@ -172,7 +172,7 @@ Object.defineProperties(Realm, { value: Sync, }, defaultPath: { - get: util.getterForProperty('defaultPath'), + get: util.getterForProperty('defaultPath', false), set: util.setterForProperty('defaultPath'), }, schemaVersion: { @@ -192,8 +192,8 @@ Object.defineProperties(Realm, { }, clearTestState: { value: function() { - collections.clearMutationListeners(); objects.clearRegisteredConstructors(); + util.invalidateCache(); rpc.clearTestState(); }, }, diff --git a/lib/browser/objects.js b/lib/browser/objects.js index bf87efbf87..4c5a05d477 100644 --- a/lib/browser/objects.js +++ b/lib/browser/objects.js @@ -19,7 +19,8 @@ 'use strict'; import { keys, objectTypes } from './constants'; -import { getterForProperty, setterForProperty, createMethods } from './util'; +import { getterForProperty, setterForProperty, createMethods, cacheObject } from './util'; +import * as rpc from './rpc' let registeredConstructors = {}; let registeredRealmPaths = {}; @@ -69,6 +70,10 @@ export function createObject(realmId, info) { throw new Error('Realm object constructor must not return another value'); } } + for (let key in info.cache) { + info.cache[key] = rpc.deserialize(undefined, info.cache[key]) + } + cacheObject(realmId, info.id, info.cache); return object; } diff --git a/lib/browser/rpc.js b/lib/browser/rpc.js index a37fee311c..823bda7ed3 100644 --- a/lib/browser/rpc.js +++ b/lib/browser/rpc.js @@ -19,6 +19,7 @@ 'use strict'; import * as base64 from './base64'; +import * as util from './util'; import { keys, objectTypes } from './constants'; const { id: idKey, realm: _realmKey } = keys; @@ -58,9 +59,16 @@ export function createSession(refreshAccessToken, host) { } function beforeNotify(realm) { - // No body required: the mere act of invoking this callback requires - // waiting for an event loop cycle in the browser and so achieves the goal - // of not letting notify() run when it wouldn't in normal usage + // NOTE: the mere existence of this function is important for read + // isolation even independent of what it does in its body. By having a + // beforenotify listener, we ensure that the RPC server can't proceed in + // notify() to autorefresh until the browser performs a callback poll. + // Without this, the RPC server could autorefresh in between two subsequent + // property reads from the browser. + + // Clear the cache for this Realm, and reenable caching if it was disabled + // by a write transaction. + util.invalidateCache(realm[keys.realm]); } export function createRealm(args) { @@ -127,6 +135,17 @@ export function callMethod(realmId, id, name, args) { return deserialize(realmId, result); } +export function getObject(realmId, id, name) { + let result = sendRequest('get_object', { realmId, id, name }); + if (!result) { + return result; + } + for (let key in result) { + result[key] = deserialize(realmId, result[key]); + } + return result; +} + export function getProperty(realmId, id, name) { let result = sendRequest('get_property', { realmId, id, name }); return deserialize(realmId, result); @@ -317,7 +336,6 @@ function sendRequest(command, data, host = sessionHost) { throw new Error(error || `Invalid response for "${command}"`); } - let callback = response.callback; if (callback != null) { let result, error, stack; try { diff --git a/lib/browser/session.js b/lib/browser/session.js index b0021ddebc..5b245a2106 100644 --- a/lib/browser/session.js +++ b/lib/browser/session.js @@ -27,9 +27,9 @@ export default class Session { } Object.defineProperties(Session.prototype, { - connectionState: { get: getterForProperty('connectionState') }, - state: { get: getterForProperty('state') }, - url: { get: getterForProperty('url') }, + connectionState: { get: getterForProperty('connectionState', false) }, + state: { get: getterForProperty('state', false) }, + url: { get: getterForProperty('url', false) }, }); createMethods(Session.prototype, objectTypes.SESSION, [ diff --git a/lib/browser/subscription.js b/lib/browser/subscription.js index b101ece6d8..da3ed538b8 100644 --- a/lib/browser/subscription.js +++ b/lib/browser/subscription.js @@ -32,18 +32,20 @@ Object.defineProperties(Subscription.prototype, { // Non-mutating methods: createMethods(Subscription.prototype, objectTypes.SUBSCRIPTION, [ - 'unsubscribe', 'addListener', 'removeListener', 'removeAllListeners' ]); +// Mutating methods: +createMethods(Subscription.prototype, objectTypes.SUBSCRIPTION, [ + 'unsubscribe', +], true); + export function createSubscription(realmId, info) { let subscription = Object.create(Subscription.prototype); - - subscription[keys.realm] = "(Subscription object)"; + subscription[keys.realm] = realmId; subscription[keys.id] = info.id; subscription[keys.type] = objectTypes.SUBSCRIPTION; - return subscription; } diff --git a/lib/browser/util.js b/lib/browser/util.js index 4c6d0c78b7..6f44bfbcad 100644 --- a/lib/browser/util.js +++ b/lib/browser/util.js @@ -18,23 +18,22 @@ 'use strict'; -import { fireMutationListeners } from './collections'; import { keys } from './constants'; import * as rpc from './rpc'; -export function createMethods(prototype, type, methodNames, mutates) { +export function createMethods(prototype, type, methodNames, mutating) { let props = {}; methodNames.forEach((name) => { props[name] = { - value: createMethod(type, name, mutates), + value: createMethod(type, name, mutating), }; }); Object.defineProperties(prototype, props); } -export function createMethod(type, name, mutates) { +export function createMethod(type, name, mutating) { return function() { let realmId = this[keys.realm]; let id = this[keys.id]; @@ -46,31 +45,72 @@ export function createMethod(type, name, mutates) { throw new TypeError(`${type}.${name} was called on Realm object of type ${this[keys.type]}!`); } + if (mutating) { + invalidateCache(realmId); + } try { return rpc.callMethod(realmId, id, name, Array.from(arguments)); - } finally { - if (mutates) { - fireMutationListeners(realmId); + } + finally { + if (mutating) { + invalidateCache(realmId); } } }; } -export function getterForProperty(name) { +let propertyCache = {}; + +export function invalidateCache(realmId) { + if (realmId) { + propertyCache[realmId] = {}; + } + else { + propertyCache = {}; + } +} + +function getRealmCache(realmId) { + let realmCache = propertyCache[realmId]; + if (!realmCache) { + realmCache = propertyCache[realmId] = {}; + } + return realmCache; +} + +export function cacheObject(realmId, id, value) { + getRealmCache(realmId)[id] = value; +} + +export function getProperty(obj, name, cache = true) { + let realmId = obj[keys.realm]; + let id = obj[keys.id]; + if (!cache || realmId === undefined) { + return rpc.getProperty(realmId, id, name); + } + + let realmCache = getRealmCache(realmId); + let objCache = realmCache[id]; + if (!objCache) { + objCache = realmCache[id] = rpc.getObject(realmId, id, name); + return objCache[name]; + } + + if (name in objCache) { + return objCache[name]; + } + return objCache[name] = rpc.getProperty(realmId, id, name); +} + +export function getterForProperty(name, cache = true) { return function() { - return rpc.getProperty(this[keys.realm], this[keys.id], name); + return getProperty(this, name, cache); }; } export function setterForProperty(name) { return function(value) { - let realmId = this[keys.realm]; - - rpc.setProperty(realmId, this[keys.id], name, value); - - // If this isn't a primitive value, then it might create a new object in the Realm. - if (value && typeof value == 'object') { - fireMutationListeners(realmId); - } + invalidateCache(this[keys.realm]); + rpc.setProperty(this[keys.realm], this[keys.id], name, value); }; } diff --git a/src/rpc.cpp b/src/rpc.cpp index 329205a56b..cc5e36c425 100644 --- a/src/rpc.cpp +++ b/src/rpc.cpp @@ -184,6 +184,58 @@ void RPCWorker::stop() { } } +static json read_object_properties(Object& object) { + json cache; + if (!object.is_valid()) { + return cache; + } + + // Send the values of the primitive and short string properties directly + // as the overhead of doing so is tiny compared to even a single RPC request + auto& object_schema = object.get_object_schema(); + auto row = object.row(); + for (auto& property : object_schema.persisted_properties) { + if (is_array(property.type)) { + continue; + } + if (is_nullable(property.type) && row.is_null(property.table_column)) { + cache[property.name] = {{"value", json(nullptr)}}; + continue; + } + auto cache_value = [&](auto&& v) { + cache[property.name] = {{"value", v}}; + }; + switch (property.type & ~PropertyType::Flags) { + case PropertyType::Bool: cache_value(row.get_bool(property.table_column)); break; + case PropertyType::Int: cache_value(row.get_int(property.table_column)); break; + case PropertyType::Float: cache_value(row.get_float(property.table_column)); break; + case PropertyType::Double: cache_value(row.get_double(property.table_column)); break; + case PropertyType::Date: { + auto ts = row.get_timestamp(property.table_column); + cache[property.name] = { + {"type", RealmObjectTypesDate}, + {"value", ts.get_seconds() * 1000.0 + ts.get_nanoseconds() / 1000000.0}, + }; + break; + } + break; + case PropertyType::String: { + auto str = row.get_string(property.table_column); + // A completely abitrary upper limit on how big of a string we'll pre-cache + if (str.size() < 100) { + cache_value(str); + } + break; + } + case PropertyType::Data: + case PropertyType::Object: + break; + default: REALM_UNREACHABLE(); + } + } + return cache; +} + RPCServer::RPCServer() { m_context = JSGlobalContextCreate(NULL); get_rpc_server(m_context) = this; @@ -359,17 +411,42 @@ RPCServer::RPCServer() { JSValueRef result = jsc::Function::call(m_context, function, object, arg_count, arg_values); return (json){{"result", serialize_json_value(result)}}; }; + m_requests["/get_object"] = [this](const json dict) -> json { + RPCObjectID oid = dict["id"].get(); + json name = dict["name"]; + JSObjectRef object = get_object(oid); + if (!object) { + return {{"result", nullptr}}; + } + + json result; + if (jsc::Object::is_instance>(m_context, object)) { + auto obj = jsc::Object::get_internal>(object); + result = read_object_properties(*obj); + } + if (result.find(name) == result.end()) { + if (name.is_number()) { + auto key = name.get(); + result[key] = serialize_json_value(jsc::Object::get_property(m_context, object, key)); + } + else { + auto key = name.get(); + result[key] = serialize_json_value(jsc::Object::get_property(m_context, object, key)); + } + } + return {{"result", result}}; + }; m_requests["/get_property"] = [this](const json dict) { RPCObjectID oid = dict["id"].get(); json name = dict["name"]; JSValueRef value; - if (JSValueRef object = get_object(oid)) { + if (JSObjectRef object = get_object(oid)) { if (name.is_number()) { - value = jsc::Object::get_property(m_context, get_object(oid), name.get()); + value = jsc::Object::get_property(m_context, object, name.get()); } else { - value = jsc::Object::get_property(m_context, get_object(oid), name.get()); + value = jsc::Object::get_property(m_context, object, name.get()); } } else { @@ -623,7 +700,8 @@ json RPCServer::serialize_json_value(JSValueRef js_value) { return { {"type", RealmObjectTypesObject}, {"id", store_object(js_object)}, - {"schema", serialize_object_schema(object->get_object_schema())} + {"schema", serialize_object_schema(object->get_object_schema())}, + {"cache", read_object_properties(*object)} }; } else if (jsc::Object::is_instance>(m_context, js_object)) { @@ -645,6 +723,7 @@ json RPCServer::serialize_json_value(JSValueRef js_value) { }; } else if (jsc::Object::is_instance>(m_context, js_object)) { + auto realm = jsc::Object::get_internal>(js_object); json realm_dict { {"_isPartialRealm", serialize_json_value(jsc::Object::get_property(m_context, js_object, "_isPartialRealm"))}, {"inMemory", serialize_json_value(jsc::Object::get_property(m_context, js_object, "inMemory"))}, @@ -655,6 +734,7 @@ json RPCServer::serialize_json_value(JSValueRef js_value) { return { {"type", RealmObjectTypesRealm}, {"id", store_object(js_object)}, + {"realmId", (uintptr_t)realm->get()}, {"data", realm_dict} }; } From 7ab1eb8c9df6e291acff4ac073e5492ffee80054 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 13 May 2019 12:44:04 -0700 Subject: [PATCH 08/18] Exclude objectstore's git repo from the npm package --- src/.npmignore | 1 + 1 file changed, 1 insertion(+) diff --git a/src/.npmignore b/src/.npmignore index c90d804ff1..759f0614a0 100644 --- a/src/.npmignore +++ b/src/.npmignore @@ -7,5 +7,6 @@ /object-store/**/*.sh /object-store/**/CMake* /object-store/**/Makefile +/object-store/.git /object-store/external/catch/ /object-store/tests/ From e41c15f0eb1d5a2f531e54b29d0aac28a1366c2c Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Mon, 13 May 2019 13:10:07 -0700 Subject: [PATCH 09/18] bump objectstore --- src/object-store | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object-store b/src/object-store index 3e48b69764..6b1b782c27 160000 --- a/src/object-store +++ b/src/object-store @@ -1 +1 @@ -Subproject commit 3e48b69764c0a2aaaa7a3b947d6d0dae215f9a09 +Subproject commit 6b1b782c27c232b1bce01ab843b1ed00375c0ff6 From 628d73d4dffa455ab09dee04cc35bdf0d9821578 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 15 May 2019 08:59:24 -0700 Subject: [PATCH 10/18] Run subscription tests on all platforms --- tests/js/subscription-tests.js | 100 ++------------------------------- 1 file changed, 4 insertions(+), 96 deletions(-) diff --git a/tests/js/subscription-tests.js b/tests/js/subscription-tests.js index 2d657e24fc..0136db9543 100644 --- a/tests/js/subscription-tests.js +++ b/tests/js/subscription-tests.js @@ -20,32 +20,9 @@ 'use strict'; -/* global REALM_MODULE_PATH */ - const Realm = require('realm'); const TestCase = require('./asserts'); -let schemas = require('./schemas'); - -const isElectronProcess = typeof process === 'object' && process.type === 'renderer'; -const isNodeProccess = typeof process === 'object' && process + '' === '[object process]' && !isElectronProcess; - -const require_method = require; -function node_require(module) { - return require_method(module); -} - -let tmp; -let fs; -let execFile; -let path; - -if (isNodeProccess) { - tmp = node_require('tmp'); - fs = node_require('fs'); - execFile = node_require('child_process').execFile; - tmp.setGracefulCleanup(); - path = node_require("path"); -} +const schemas = require('./schemas'); function uuid() { return 'xxxxxxxx-xxxx-4xxx-yxxx-xxxxxxxxxxxx'.replace(/[xy]/g, function (c) { @@ -144,10 +121,6 @@ function verifySubscriptionWithParents(parentToInclude, filterClause) { module.exports = { testSubscriptionWrapperProperties() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { return new Promise((resolve, reject) => { const subscription = realm.objects("ObjectA").subscribe("test"); @@ -159,10 +132,6 @@ module.exports = { }, testNamedSubscriptionProperties() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { return new Promise((resolve, reject) => { const now = new Date(); @@ -188,10 +157,6 @@ module.exports = { }, testUpdateQuery: function () { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { return new Promise((resolve, reject) => { const sub = realm.objects("ObjectA").filtered("name = 'Foo'").subscribe("update-named-sub-query"); @@ -206,7 +171,7 @@ module.exports = { // Updating the query must either be a string or a Results objects TestCase.assertThrows(() => namedSub.query = 0); TestCase.assertThrows(() => namedSub.query = true); - + // Updating the query using a string namedSub.query = "truepredicate"; TestCase.assertEqual(namedSub.query, "truepredicate"); @@ -214,7 +179,7 @@ module.exports = { TestCase.assertEqual(namedSub.error, undefined); TestCase.assertTrue(updated.getTime() < namedSub.updatedAt.getTime()); updated = namedSub.updatedAt; - + setTimeout(function() { // Updating the query using a Results object namedSub.query = realm.objects('ObjectA').filtered('name = "Bar"'); @@ -230,10 +195,6 @@ module.exports = { }, testUpdateTtl() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { const sub = realm.objects("ObjectA").filtered("name = 'Foo'").subscribe("update-named-sub-query"); return new Promise((resolve, reject) => { @@ -259,10 +220,6 @@ module.exports = { }, testUpdateReadOnlyProperties() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { return new Promise((resolve, reject) => { const sub = realm.objects("ObjectA").subscribe("read-only-test"); @@ -284,10 +241,6 @@ module.exports = { }, testSubscribeWithTtl() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { return new Promise((resolve, reject) => { const now = new Date(); @@ -310,9 +263,6 @@ module.exports = { }, testSubscribeAndUpdateQuery() { - if (!isNodeProccess) { - return; - } return getRealm().then(realm => { return new Promise((resolve, reject) => { let query1 = realm.objects("ObjectA"); @@ -333,7 +283,7 @@ module.exports = { resolve(); } }); - }, 2); + }, 2); } }); }); @@ -341,10 +291,6 @@ module.exports = { }, testSubscribeAndUpdateTtl() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { const query1 = realm.objects("ObjectA"); @@ -377,10 +323,6 @@ module.exports = { }, testSubscribeWithoutName() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { return new Promise((resolve, reject) => { let query = realm.objects("ObjectA"); @@ -391,10 +333,6 @@ module.exports = { }, testSubscribeWithMisspelledConfigParameter() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { return new Promise((resolve, reject) => { let query = realm.objects("ObjectA"); @@ -405,10 +343,6 @@ module.exports = { }, testSubscribeToChildrenWithoutParents() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { realm.write(() => { let obj_a1 = realm.create('ObjectA', {name: "a1"}); @@ -438,10 +372,6 @@ module.exports = { }, testSubscribeParentsWithForwardLinks() { - if (!isNodeProccess) { - return; - } - return getRealm().then(realm => { realm.write(() => { let obj_a1 = realm.create('ObjectA', {name: "a1"}); @@ -471,23 +401,14 @@ module.exports = { }, testSubscribeToChildrenWithNamedParents() { - if (!isNodeProccess) { - return; - } return verifySubscriptionWithParents("parents"); }, testSubscribeToChildrenWithUnnamedParents() { - if (!isNodeProccess) { - return; - } return verifySubscriptionWithParents("@links.Parent.child"); }, testSubscribeToChildrenWithMalformedInclusion1() { - if (!isNodeProccess) { - return; - } return verifySubscriptionWithParents("something.wrong").then(() => { throw new Error('subscription should have failed') }, @@ -496,9 +417,6 @@ module.exports = { }, testSubscribeToChildrenWithMalformedInclusion2() { - if (!isNodeProccess) { - return; - } return verifySubscriptionWithParents("@links.Parent.missing_property").then(() => { throw new Error('subscription should have failed') }, @@ -507,9 +425,6 @@ module.exports = { }, testSubscribeToChildrenWithMalformedInclusion3() { - if (!isNodeProccess) { - return; - } return verifySubscriptionWithParents(4.2).then(() => { throw new Error('subscription should have failed') }, @@ -521,17 +436,10 @@ module.exports = { // but it should not be encouraged nor documented. It is mostly to enable users to run // subscription queries that are directly copied from Studio. testSubscribeWithManualInclusion1() { - if (!isNodeProccess) { - return; - } return verifySubscriptionWithParents("", "TRUEPREDICATE INCLUDE(@links.Parent.child)"); }, testSubscribeWithManualInclusion2() { - if (!isNodeProccess) { - return; - } return verifySubscriptionWithParents("", "TRUEPREDICATE INCLUDE(parents)"); }, - }; From c8df8cd4be24e3bc7f512af27d9996206f95a2bb Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 15 May 2019 09:20:20 -0700 Subject: [PATCH 11/18] Inject the query-based sync schema when running in Chrome --- lib/browser/index.js | 16 +++++++++++++++- tests/js/realm-tests.js | 8 +------- 2 files changed, 16 insertions(+), 8 deletions(-) diff --git a/lib/browser/index.js b/lib/browser/index.js index bb9932f7bd..7cf1eef6d1 100644 --- a/lib/browser/index.js +++ b/lib/browser/index.js @@ -75,9 +75,23 @@ function getObjectType(realm, type) { export default class Realm { constructor(config) { - let schemas = typeof config == 'object' && config.schema; + let schemas = typeof config === 'object' && config.schema; let constructors = schemas ? {} : null; + let isPartial = false; + if (config && typeof config.sync === 'object') { + if (typeof config.sync.fullSynchronization !== 'undefined') { + isPartial = !config.sync.fullSynchronization; + } + else if (typeof config.sync.partial !== 'undefined') { + isPartial = config.sync.partial; + } + } + + if (schemas && isPartial) { + Realm._extendQueryBasedSchema(schemas); + } + for (let i = 0, len = schemas ? schemas.length : 0; i < len; i++) { let item = schemas[i]; diff --git a/tests/js/realm-tests.js b/tests/js/realm-tests.js index 62167f9d1b..2a75ba239c 100644 --- a/tests/js/realm-tests.js +++ b/tests/js/realm-tests.js @@ -44,7 +44,6 @@ const schemas = require('./schemas'); let pathSeparator = '/'; const isNodeProcess = typeof process === 'object' && process + '' === '[object process]'; -const isChromeWorker = !isNodeProcess && typeof WorkerGlobalScope !== 'undefined' && navigator instanceof WorkerNavigator; if (isNodeProcess && process.platform === 'win32') { pathSeparator = '\\'; } @@ -1520,12 +1519,7 @@ module.exports = { .then(user1 => { config.sync.user = user1; const realm = new Realm(config); - if (isChromeWorker) { - TestCase.assertEqual(realm.schema.length, 1); // 1 test object - } - else { - TestCase.assertEqual(realm.schema.length, 7); // 5 permissions, 1 results set, 1 test object - } + TestCase.assertEqual(realm.schema.length, 7); // 5 permissions, 1 results set, 1 test object return closeAfterUpload(realm); }) .then(() => { From 7c5de867c1026f78a6f904333e43de02be913eb7 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 15 May 2019 09:25:20 -0700 Subject: [PATCH 12/18] Expose Results.description() in the RPC wrapper --- lib/browser/results.js | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/browser/results.js b/lib/browser/results.js index 629442841b..1deb242d64 100644 --- a/lib/browser/results.js +++ b/lib/browser/results.js @@ -27,6 +27,7 @@ export default class Results extends Collection { // Non-mutating methods: createMethods(Results.prototype, objectTypes.RESULTS, [ + 'description', 'filtered', 'sorted', 'snapshot', From f4f235622213c4d6eca10341e2a66be34cae9518 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 15 May 2019 09:44:57 -0700 Subject: [PATCH 13/18] Fix some tests which assumed that subscriptions would always hit the Pending state --- tests/js/subscription-tests.js | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/tests/js/subscription-tests.js b/tests/js/subscription-tests.js index 0136db9543..8b8b3a1c55 100644 --- a/tests/js/subscription-tests.js +++ b/tests/js/subscription-tests.js @@ -65,6 +65,10 @@ function getRealm() { }); } +function pendingOrComplete(state) { + return state === Realm.Sync.SubscriptionState.Pending || state === Realm.Sync.SubscriptionState.Complete; +} + function verifySubscriptionWithParents(parentToInclude, filterClause) { return getRealm().then(realm => { realm.write(() => { @@ -138,11 +142,11 @@ module.exports = { const now_plus_2_sec = new Date(now.getTime() + 2000); const sub = realm.objects("ObjectA").subscribe("named-test"); sub.addListener((subscription, state) => { - if (state === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state)) { sub.removeAllListeners(); const namedSub = realm.subscriptions("named-test")[0]; TestCase.assertEqual(namedSub.name, "named-test"); - TestCase.assertEqual(namedSub.state, Realm.Sync.SubscriptionState.Pending); + TestCase.assertTrue(pendingOrComplete(namedSub.state)); TestCase.assertEqual(namedSub.error, undefined); TestCase.assertEqual(namedSub.objectType, "ObjectA"); TestCase.assertTrue(namedSub.createdAt.getTime() >= now.getTime() && namedSub.createdAt.getTime() < now_plus_2_sec.getTime()); @@ -175,7 +179,7 @@ module.exports = { // Updating the query using a string namedSub.query = "truepredicate"; TestCase.assertEqual(namedSub.query, "truepredicate"); - TestCase.assertEqual(namedSub.state, Realm.Sync.SubscriptionState.Pending); + TestCase.assertTrue(pendingOrComplete(namedSub.state)); TestCase.assertEqual(namedSub.error, undefined); TestCase.assertTrue(updated.getTime() < namedSub.updatedAt.getTime()); updated = namedSub.updatedAt; @@ -199,7 +203,7 @@ module.exports = { const sub = realm.objects("ObjectA").filtered("name = 'Foo'").subscribe("update-named-sub-query"); return new Promise((resolve, reject) => { sub.addListener((subscription, state) => { - if (state === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state)) { sub.removeAllListeners(); const namedSub = realm.subscriptions("update-named-sub-query")[0]; let updated = namedSub.updatedAt; @@ -224,7 +228,7 @@ module.exports = { return new Promise((resolve, reject) => { const sub = realm.objects("ObjectA").subscribe("read-only-test"); sub.addListener((subscription, state) => { - if (state === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state)) { sub.removeAllListeners(); const namedSub = realm.subscriptions("read-only-test")[0]; TestCase.assertThrows(() => namedSub.name = "Foo"); @@ -248,7 +252,7 @@ module.exports = { const query = realm.objects("ObjectA"); const sub = query.subscribe({ name: "with-ttl", timeToLive: 1000}); sub.addListener((subscription, state) => { - if (state === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state)) { sub.removeAllListeners(); const namedSub = realm.subscriptions("with-ttl")[0]; TestCase.assertTrue(now.getTime() <= namedSub.createdAt.getTime() && namedSub.createdAt.getTime() < now_plus_2_sec.getTime()); @@ -268,7 +272,7 @@ module.exports = { let query1 = realm.objects("ObjectA"); const sub1 = query1.subscribe("update-query"); sub1.addListener((subscription1, state1) => { - if (state1 === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state1)) { sub1.removeAllListeners(); const namedSub = realm.subscriptions("update-query")[0]; const update1 = namedSub.updatedAt; @@ -276,7 +280,7 @@ module.exports = { let query2 = realm.objects('ObjectA').filtered("name = 'Foo'"); const sub2 = query2.subscribe({name: 'update-query', update: true}); sub2.addListener((subscription2, state2) => { - if (state2 === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state2)) { sub2.removeAllListeners(); TestCase.assertFalse(query1.description() === query2.description()); TestCase.assertTrue(update1.getTime() < namedSub.updatedAt.getTime()); @@ -297,7 +301,7 @@ module.exports = { return new Promise((resolve, reject) => { const sub1 = query1.subscribe({name: "update-query", timeToLive: 1000}); sub1.addListener((subscription1, state1) => { - if (state1 === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state1)) { sub1.removeAllListeners(); const namedSub = realm.subscriptions("update-query")[0]; const update1 = namedSub.updatedAt; @@ -306,7 +310,7 @@ module.exports = { setTimeout(function() { const sub2 = query1.subscribe({name: 'update-query', update: true, timeToLive: 5000}); sub2.addListener((subscription2, state2) => { - if (state2 === Realm.Sync.SubscriptionState.Pending) { + if (pendingOrComplete(state2)) { sub2.removeAllListeners(); TestCase.assertTrue(update1.getTime() < namedSub.updatedAt.getTime()); TestCase.assertTrue(expires1.getTime() < namedSub.expiresAt.getTime()); From 060893c92b96eb36d926f0f808222230a044c167 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 15 May 2019 09:46:22 -0700 Subject: [PATCH 14/18] Remove some pointless promises in subscription tests --- tests/js/subscription-tests.js | 73 +++++++++++++++------------------- 1 file changed, 31 insertions(+), 42 deletions(-) diff --git a/tests/js/subscription-tests.js b/tests/js/subscription-tests.js index 8b8b3a1c55..b7bc400bfd 100644 --- a/tests/js/subscription-tests.js +++ b/tests/js/subscription-tests.js @@ -34,34 +34,32 @@ function uuid() { function getRealm() { const AUTH_URL = 'http://127.0.0.1:9080'; const REALM_URL = 'realm://127.0.0.1:9080/~/' + uuid().replace("-", "_"); - return new Promise((resolve, reject) => { - Realm.Sync.User.login(AUTH_URL, Realm.Sync.Credentials.nickname("admin", true)) - .then((user) => { - const schemas = [ - { - name: 'Parent', - properties: { - name: { type: 'string' }, - child: 'ObjectA', - } - }, - { - name: 'ObjectA', - properties: { - name: { type: 'string' }, - parents: { type: 'linkingObjects', objectType: 'Parent', property: 'child' }, - } - }, - ]; - - const config = user.createConfiguration({ - schema: schemas, - sync: { - url: REALM_URL, + return Realm.Sync.User.login(AUTH_URL, Realm.Sync.Credentials.nickname("admin", true)) + .then((user) => { + const schemas = [ + { + name: 'Parent', + properties: { + name: { type: 'string' }, + child: 'ObjectA', } - }); - resolve(new Realm(config)); + }, + { + name: 'ObjectA', + properties: { + name: { type: 'string' }, + parents: { type: 'linkingObjects', objectType: 'Parent', property: 'child' }, + } + }, + ]; + + const config = user.createConfiguration({ + schema: schemas, + sync: { + url: REALM_URL, + } }); + return new Realm(config); }); } @@ -126,12 +124,9 @@ module.exports = { testSubscriptionWrapperProperties() { return getRealm().then(realm => { - return new Promise((resolve, reject) => { - const subscription = realm.objects("ObjectA").subscribe("test"); - TestCase.assertEqual(subscription.name, "test"); - TestCase.assertEqual(subscription.state, Realm.Sync.SubscriptionState.Creating); - resolve(); - }); + const subscription = realm.objects("ObjectA").subscribe("test"); + TestCase.assertEqual(subscription.name, "test"); + TestCase.assertEqual(subscription.state, Realm.Sync.SubscriptionState.Creating); }); }, @@ -328,21 +323,15 @@ module.exports = { testSubscribeWithoutName() { return getRealm().then(realm => { - return new Promise((resolve, reject) => { - let query = realm.objects("ObjectA"); - query.subscribe({ update: true, timeToLive: 1000}); // Missing name, doesn't throw - resolve(); - }); + let query = realm.objects("ObjectA"); + query.subscribe({ update: true, timeToLive: 1000}); // Missing name, doesn't throw }); }, testSubscribeWithMisspelledConfigParameter() { return getRealm().then(realm => { - return new Promise((resolve, reject) => { - let query = realm.objects("ObjectA"); - TestCase.assertThrowsContaining(() => query.subscribe({ naem: "myName" }), "Unexpected property in subscription options: 'naem'"); - resolve(); - }); + let query = realm.objects("ObjectA"); + TestCase.assertThrowsContaining(() => query.subscribe({ naem: "myName" }), "Unexpected property in subscription options: 'naem'"); }); }, From ab503c9d58f99aebe6e2235790c25281f5768efa Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 15 May 2019 11:41:10 -0700 Subject: [PATCH 15/18] Update changelog --- CHANGELOG.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b5d0ada17..a7742cd0eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,22 @@ +x.x.x Release notes (yyyy-MM-dd) +============================================================= +### Enhancements +* Improve performance when using Chrome Debugging with React Native by adding caching and reducing the number of RPC calls required. Read-heavy workflows are as much as 10x faster. Write-heavy workflows will see a much smaller improvement, but also had a smaller performance hit to begin with. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). + +### Fixed +* ([#????](https://github.com/realm/realm-js/issues/????), since v?.?.?) +* Opening a query-based Realm using `new Realm` did not automatically add the required types to the schema when running in Chrome, resulting in errors when trying to manage subscriptions. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). +* The Chrome debugger did not properly enforce read isolation, meaning that reading a property twice in a row could produce different values if another thread performed a write in between the reads. This was typically only relevant to synchronized Realms due to the lack of multithreading support in the supported Javascript environments. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). +* The RPC server for Chrome debugging would sometimes deadlock if a notification fired at the same time as a Realm function which takes a callback was called. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). + +### Compatibility +* Realm Object Server: 3.11.0 or later. +* APIs are backwards compatible with all previous release of realm in the 2.x.y series. +* File format: Generates Realms with format v9 (Reads and upgrades all previous formats) + +### Internal +* None. + 2.27.0 Release notes (2019-5-15) ============================================================= NOTE: The minimum version of Realm Object Server has been increased to 3.21.0 and attempting to connect to older versions will produce protocol mismatch errors. Realm Cloud has already been upgraded to this version, and users using that do not need to worry about this. From 634356594a33ae7928f528db9b0b2ace65bd31c8 Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Wed, 15 May 2019 16:20:10 -0700 Subject: [PATCH 16/18] Move HANDLESCOPE to the correct place for schema change notifications --- src/js_realm.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/js_realm.hpp b/src/js_realm.hpp index 62bbc6d165..375458648f 100644 --- a/src/js_realm.hpp +++ b/src/js_realm.hpp @@ -69,15 +69,18 @@ template class RealmDelegate : public BindingContext { private: void did_change(std::vector const&, std::vector const&, bool) override { + HANDLESCOPE notify(m_notifications, "change"); } void schema_did_change(realm::Schema const& schema) override { + HANDLESCOPE ObjectType schema_object = Schema::object_for_schema(m_context, schema); notify(m_schema_notifications, "schema", schema_object); } void before_notify() override { + HANDLESCOPE notify(m_before_notify_notifications, "beforenotify"); } @@ -168,8 +171,6 @@ class RealmDelegate : public BindingContext { // from inside the handler template void notify(std::list> notifications, const char *name, Args&&... args) { - HANDLESCOPE - auto realm = m_realm.lock(); if (!realm) { throw std::runtime_error("Realm no longer exists"); From 1c0bb87534c51188b6af5639c70650026cdca70f Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 16 May 2019 09:55:52 -0700 Subject: [PATCH 17/18] Changelog improvements --- CHANGELOG.md | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a7742cd0eb..b8e3242ada 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,16 +1,16 @@ x.x.x Release notes (yyyy-MM-dd) ============================================================= ### Enhancements -* Improve performance when using Chrome Debugging with React Native by adding caching and reducing the number of RPC calls required. Read-heavy workflows are as much as 10x faster. Write-heavy workflows will see a much smaller improvement, but also had a smaller performance hit to begin with. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). +* Improve performance when using Chrome Debugging with React Native by adding caching and reducing the number of RPC calls required. Read-heavy workflows are as much as 10x faster. Write-heavy workflows will see a much smaller improvement, but also had a smaller performance hit to begin with. (Issue: [#491](https://github.com/realm/realm-js/issues/491), PR: [#2373](https://github.com/realm/realm-js/pull/2373)). ### Fixed * ([#????](https://github.com/realm/realm-js/issues/????), since v?.?.?) -* Opening a query-based Realm using `new Realm` did not automatically add the required types to the schema when running in Chrome, resulting in errors when trying to manage subscriptions. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). -* The Chrome debugger did not properly enforce read isolation, meaning that reading a property twice in a row could produce different values if another thread performed a write in between the reads. This was typically only relevant to synchronized Realms due to the lack of multithreading support in the supported Javascript environments. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). -* The RPC server for Chrome debugging would sometimes deadlock if a notification fired at the same time as a Realm function which takes a callback was called. (PR: ([#2373](https://github.com/realm/realm-js/pull/2373))). +* Opening a query-based Realm using `new Realm` did not automatically add the required types to the schema when running in Chrome, resulting in errors when trying to manage subscriptions. (PR: [#2373](https://github.com/realm/realm-js/pull/2373), since v2.15.0). +* The Chrome debugger did not properly enforce read isolation, meaning that reading a property twice in a row could produce different values if another thread performed a write in between the reads. This was typically only relevant to synchronized Realms due to the lack of multithreading support in the supported Javascript environments. (PR: [#2373](https://github.com/realm/realm-js/pull/2373), since v1.0.0). +* The RPC server for Chrome debugging would sometimes deadlock if a notification fired at the same time as a Realm function which takes a callback was called. (PR: [#2373](https://github.com/realm/realm-js/pull/2373), since v1.0.0 in various forms). ### Compatibility -* Realm Object Server: 3.11.0 or later. +* Realm Object Server: 3.21.0 or later. * APIs are backwards compatible with all previous release of realm in the 2.x.y series. * File format: Generates Realms with format v9 (Reads and upgrades all previous formats) From db8f0334079c17dad185a4a665a04ab1964c2bda Mon Sep 17 00:00:00 2001 From: Thomas Goyne Date: Thu, 16 May 2019 09:56:22 -0700 Subject: [PATCH 18/18] Bump objectstore --- src/object-store | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/object-store b/src/object-store index 6b1b782c27..3354b3f953 160000 --- a/src/object-store +++ b/src/object-store @@ -1 +1 @@ -Subproject commit 6b1b782c27c232b1bce01ab843b1ed00375c0ff6 +Subproject commit 3354b3f95351fcc88d8671017b45b9dbf3673886