Merge pull request #100 from realm/al-notification

improve notification api

lib/constants.js

@@ -15,7 +15,6 @@ let propTypes = {};
 
 [
     'FUNCTION',
-    'NOTIFICATION',
     'REALM',
     'RESULTS',
 ].forEach(function(type) {

lib/notifications.js

@@ -1,21 +0,0 @@
-'use strict';
-
-const constants = require('./constants');
-
-const {keys} = constants;
-
-module.exports = {
-    create,
-};
-
-class Notification {}
-
-function create(realmId, info) {
-    let notification = new Notification();
-
-    notification[keys.realm] = realmId;
-    notification[keys.id] = info.id;
-    notification[keys.type] = info.type;
-
-    return notification;
-}

lib/realm.js

@@ -3,21 +3,18 @@
 const constants = require('./constants');
 const lists = require('./lists');
 const objects = require('./objects');
-const notifications = require('./notifications');
 const results = require('./results');
 const rpc = require('./rpc');
 const util = require('./util');
 
 const {keys, propTypes, objectTypes} = constants;
-const notificationsKey = Symbol();
+const listenersKey = Symbol();
-const notificationCallbackKey = Symbol();
 const resultsKey = Symbol();
 
 // TODO: DATA
 rpc.registerTypeConverter(propTypes.DATE, (_, info) => new Date(info.value));
 rpc.registerTypeConverter(propTypes.LIST, lists.create);
 rpc.registerTypeConverter(propTypes.OBJECT, objects.create);
-rpc.registerTypeConverter(objectTypes.NOTIFICATION, notifications.create);
 rpc.registerTypeConverter(objectTypes.RESULTS, results.create);
 
 class Realm {
@@ -42,7 +39,7 @@ class Realm {
         this[keys.id] = realmId;
         this[keys.realm] = realmId;
         this[keys.type] = objectTypes.REALM;
-        this[notificationsKey] = [];
+        this[listenersKey] = [];
         this[resultsKey] = [];
 
         [
@@ -53,18 +50,35 @@ class Realm {
         });
     }
 
-    addNotification(callback) {
+    addListener(name, callback) {
         if (typeof callback != 'function') {
-            throw new Error('Realm.addNotification must be passed a function!');
+            throw new Error('Realm.addListener must be passed a function!');
         }
+        if (name != 'change') { 
+            throw new Error("Only 'change' notification is supported.");
+        }
+        this[listenersKey].push(callback);
+    }
 
-        let method = util.createMethod(objectTypes.REALM, 'addNotification');
-        let notification = method.apply(this, arguments);
 
-        notification[notificationCallbackKey] = callback;
+    removeListener(name, callback) {
+        if (typeof callback != 'function') {
+            throw new Error('Realm.addListener must be passed a function!');
+        }
+        if (name != 'change') { 
+            throw new Error("Only 'change' notification is supported.");
+        }
+        var index = 0;
+        while((index = this[listenersKey].indexOf(callback, index)) != -1) {
+            this[listenersKey].splice(index, 1);
+        };
+    }
 
-        this[notificationsKey].push(notification);
+    removeAllListeners(name) {
-        return notification;
+        if (name != undefined && name != 'change') { 
+            throw new Error("Only 'change' notification is supported.");
+        }
+        this[listenersKey] = [];
     }
 
     objects() {
@@ -100,9 +114,8 @@ class Realm {
             results[keys.resize]();
         }
 
-        for (let notification of this[notificationsKey]) {
+        for (let callback of this[listenersKey]) {
-            let callback = notification[notificationCallbackKey];
+            callback(this, 'change');
-            callback(this, 'DidChangeNotification');
         }
     }
 }

src/RJSRealm.hpp

@@ -26,7 +26,6 @@ namespace realm {
 
 JSClassRef RJSRealmClass();
 JSClassRef RJSRealmConstructorClass();
-JSClassRef RJSNotificationClass();
 
 std::string RJSDefaultPath();
 void RJSSetDefaultPath(std::string path);

src/RJSRealm.mm

@@ -32,43 +32,25 @@ using namespace realm;
 
 class RJSRealmDelegate : public RealmDelegate {
 public:
-    typedef std::shared_ptr<std::function<void(const std::string)>> NotificationFunction;
-    void add_notification(NotificationFunction &notification) { m_notifications.insert(notification); }
-    void remove_notification(NotificationFunction notification) { m_notifications.erase(notification); }
-    void remove_all_notifications() { m_notifications.clear(); }
-    std::set<NotificationFunction> m_notifications;
-
     virtual void changes_available() {
-        for (NotificationFunction notification : m_notifications) {
+        assert(0);
-            (*notification)("RefreshRequiredNotification");
-        }
     }
-
+    virtual void did_change(std::vector<ObserverState> const& observers, std::vector<void*> const& invalidated) {
-    virtual void did_change(std::vector<ObserverState> const& observers,
+        notify("change");
-                            std::vector<void*> const& invalidated) {
-        for (NotificationFunction notification : m_notifications) {
-            (*notification)("DidChangeNotification");
-        }
     }
-
     virtual std::vector<ObserverState> get_observed_rows() {
         return std::vector<ObserverState>();
     }
-
     virtual void will_change(std::vector<ObserverState> const& observers,
-                             std::vector<void*> const& invalidated) {
+                             std::vector<void*> const& invalidated) {}
 
-    }
+    RJSRealmDelegate(WeakRealm realm, JSGlobalContextRef ctx) : m_context(ctx), m_realm(realm) {
-
-    JSGlobalContextRef m_context;
-    std::map<std::string, ObjectDefaults> m_defaults;
-    std::map<std::string, JSValueRef> m_prototypes;
-
-    RJSRealmDelegate(JSGlobalContextRef ctx) : m_context(ctx) {
         JSGlobalContextRetain(m_context);
     }
 
     ~RJSRealmDelegate() {
+        remove_all_notifications();
+
         for (auto prototype : m_prototypes) {
             JSValueUnprotect(m_context, prototype.second);
         }
@@ -79,6 +61,48 @@ public:
         }
         JSGlobalContextRelease(m_context);
     }
+
+    void add_notification(JSObjectRef notification) {
+        JSValueProtect(m_context, notification);
+        m_notifications.insert(notification);
+    }
+    void remove_notification(JSObjectRef notification) {
+        JSValueUnprotect(m_context, notification);
+        m_notifications.erase(notification);
+    }
+    void remove_all_notifications() {
+        for (auto notification : m_notifications) {
+            JSValueUnprotect(m_context, notification);
+        }
+        m_notifications.clear();
+    }
+
+    std::map<std::string, ObjectDefaults> m_defaults;
+    std::map<std::string, JSValueRef> m_prototypes;
+
+  private:
+    std::set<JSObjectRef> m_notifications;
+    JSGlobalContextRef m_context;
+    WeakRealm m_realm;
+
+    void notify(const char *notification_name) {
+        JSValueRef arguments[2];
+        SharedRealm realm = m_realm.lock();
+        if (!realm) {
+            throw std::runtime_error("Realm no longer exists");
+        }
+        JSObjectRef realm_object = RJSWrapObject<SharedRealm *>(m_context, RJSRealmClass(), new SharedRealm(realm));
+        arguments[0] = realm_object;
+        arguments[1] = RJSValueForString(m_context, notification_name);
+
+        for (auto callback : m_notifications) {
+            JSValueRef ex = NULL;
+            JSObjectCallAsFunction(m_context, callback, realm_object, 2, arguments, &ex);
+            if (ex) {
+                throw RJSException(m_context, ex);
+            }
+        }
+    }
 };
 
 std::map<std::string, ObjectDefaults> &RJSDefaults(Realm *realm) {
@@ -189,7 +213,7 @@ JSObjectRef RealmConstructor(JSContextRef ctx, JSObjectRef constructor, size_t a
         }
         SharedRealm realm = Realm::get_shared_realm(config);
         if (!realm->m_delegate) {
-            realm->m_delegate = std::make_unique<RJSRealmDelegate>(JSContextGetGlobalContext(ctx));
+            realm->m_delegate = std::make_unique<RJSRealmDelegate>(realm, JSContextGetGlobalContext(ctx));
         }
         RJSDefaults(realm.get()) = defaults;
         RJSPrototypes(realm.get()) = prototypes;
@@ -399,48 +423,60 @@ JSValueRef RealmWrite(JSContextRef ctx, JSObjectRef function, JSObjectRef thisOb
     return NULL;
 }
 
-namespace realm {
+std::string RJSValidatedNotificationName(JSContextRef ctx, JSValueRef value) {
-    struct Notification {
+    std::string name = RJSValidatedStringForValue(ctx, value);
-        JSGlobalContextRef ctx;
+    if (name != "change") {
-        JSObjectRef realmObject;
+        throw std::runtime_error("Only the 'change' notification name is supported.");
-        JSObjectRef callbackObject;
+    }
-        RJSRealmDelegate::NotificationFunction func;
+    return name;
-
-        Notification(JSGlobalContextRef c, JSObjectRef r, JSObjectRef cb, RJSRealmDelegate::NotificationFunction f) : ctx(c), realmObject(r), callbackObject(cb), func(f) {
-            JSGlobalContextRetain(ctx);
-            JSValueProtect(ctx, realmObject);
-            JSValueProtect(ctx, callbackObject);
-        }
-
-        ~Notification() {
-            JSValueUnprotect(ctx, callbackObject);
-            JSValueUnprotect(ctx, realmObject);
-            JSGlobalContextRelease(ctx);
-        }
-    };
 }
 
-JSValueRef RealmAddNotification(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* jsException) {
+JSValueRef RealmAddListener(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* jsException) {
     try {
-        RJSValidateArgumentCount(argumentCount, 1);
+        RJSValidateArgumentCount(argumentCount, 2);
+        __unused std::string name = RJSValidatedNotificationName(ctx, arguments[0]);
+        JSObjectRef callback = RJSValidatedValueToFunction(ctx, arguments[1]);
 
-        JSObjectRef callback = RJSValidatedValueToFunction(ctx, arguments[0]);
         SharedRealm realm = *RJSGetInternal<SharedRealm *>(thisObject);
-        JSGlobalContextRef gCtx = JSContextGetGlobalContext(ctx);
+        static_cast<RJSRealmDelegate *>(realm->m_delegate.get())->add_notification(callback);
+        return NULL;
+    }
+    catch (std::exception &exp) {
+        if (jsException) {
+            *jsException = RJSMakeError(ctx, exp);
+        }
+        return NULL;
+    }
+}
 
-        RJSRealmDelegate::NotificationFunction func = std::make_shared<std::function<void(const std::string)>>([=](std::string notification_name) {
+JSValueRef RealmRemoveListener(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* jsException) {
-            JSValueRef arguments[2];
+    try {
-            arguments[0] = thisObject;
+        RJSValidateArgumentCount(argumentCount, 2);
-            arguments[1] = RJSValueForString(gCtx, notification_name);
+        __unused std::string name = RJSValidatedNotificationName(ctx, arguments[0]);
-            JSValueRef ex = NULL;
+        JSObjectRef callback = RJSValidatedValueToFunction(ctx, arguments[1]);
-            JSObjectCallAsFunction(gCtx, callback, thisObject, 2, arguments, &ex);
-            if (ex) {
-                throw RJSException(gCtx, ex);
-            }
-        });
 
-        static_cast<RJSRealmDelegate *>(realm->m_delegate.get())->add_notification(func);
+        SharedRealm realm = *RJSGetInternal<SharedRealm *>(thisObject);
-        return RJSWrapObject<Notification *>(ctx, RJSNotificationClass(), new Notification { gCtx, thisObject, callback, func });
+        static_cast<RJSRealmDelegate *>(realm->m_delegate.get())->remove_notification(callback);
+        return NULL;
+    }
+    catch (std::exception &exp) {
+        if (jsException) {
+            *jsException = RJSMakeError(ctx, exp);
+        }
+        return NULL;
+    }
+}
+
+JSValueRef RealmRemoveAllListeners(JSContextRef ctx, JSObjectRef function, JSObjectRef thisObject, size_t argumentCount, const JSValueRef arguments[], JSValueRef* jsException) {
+    try {
+        RJSValidateArgumentRange(argumentCount, 0, 1);
+        if (argumentCount) {
+            RJSValidatedNotificationName(ctx, arguments[0]);
+        }
+
+        SharedRealm realm = *RJSGetInternal<SharedRealm *>(thisObject);
+        static_cast<RJSRealmDelegate *>(realm->m_delegate.get())->remove_all_notifications();
+        return NULL;
     }
     catch (std::exception &exp) {
         if (jsException) {
@@ -472,7 +508,9 @@ static const JSStaticFunction RJSRealmFuncs[] = {
     {"delete", RealmDelete, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
     {"deleteAll", RealmDeleteAll, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
     {"write", RealmWrite, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
-    {"addNotification", RealmAddNotification, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
+    {"addListener", RealmAddListener, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
+    {"removeListener", RealmRemoveListener, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
+    {"removeAllListeners", RealmRemoveAllListeners, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
     {"close", RealmClose, kJSPropertyAttributeReadOnly | kJSPropertyAttributeDontEnum | kJSPropertyAttributeDontDelete},
     {NULL, NULL},
 };
@@ -481,9 +519,3 @@ JSClassRef RJSRealmClass() {
     static JSClassRef s_realmClass = RJSCreateWrapperClass<SharedRealm *>("Realm", RealmGetProperty, NULL, RJSRealmFuncs);
     return s_realmClass;
 }
-
-JSClassRef RJSNotificationClass() {
-    static JSClassRef s_notificationClass = RJSCreateWrapperClass<Notification *>("Notification", NULL, NULL, NULL);
-    return s_notificationClass;
-}
-

src/RealmRPC.cpp

@@ -254,12 +254,6 @@ json RPCServer::serialize_json_value(JSValueRef value) {
             {"schema", serialize_object_schema(results->object_schema)}
         };
     }
-    else if (JSValueIsObjectOfClass(m_context, value, RJSNotificationClass())) {
-        return {
-            {"type", RealmObjectTypesNotification},
-            {"id", store_object(js_object)},
-        };
-    }
     else if (RJSIsValueArray(m_context, value)) {
         size_t length = RJSValidatedListLength(m_context, js_object);
         std::vector<json> array;

tests/RealmTests.js

@@ -282,7 +282,7 @@ module.exports = BaseTest.extend({
         var notificationCount = 0;
         var notificationName;
 
-        var notification = realm.addNotification(function(realm, name) {
+        realm.addListener('change', function(realm, name) {
             notificationCount++;
             notificationName = name;
         });
@@ -290,6 +290,38 @@ module.exports = BaseTest.extend({
         TestCase.assertEqual(notificationCount, 0);
         realm.write(function() {});
         TestCase.assertEqual(notificationCount, 1);
-        TestCase.assertEqual(notificationName, 'DidChangeNotification');
+        TestCase.assertEqual(notificationName, 'change');
+
+        var secondNotificationCount = 0;
+        function secondNotification(realm, name) {
+            secondNotificationCount++;
+        };
+        realm.addListener('change', secondNotification)
+
+        realm.write(function() {});
+        TestCase.assertEqual(notificationCount, 2);
+        TestCase.assertEqual(secondNotificationCount, 1);
+
+        realm.removeListener('change', secondNotification);
+        realm.write(function() {});
+        TestCase.assertEqual(notificationCount, 3);
+        TestCase.assertEqual(secondNotificationCount, 1);
+
+        realm.removeAllListeners();
+        realm.write(function() {});
+        TestCase.assertEqual(notificationCount, 3);
+        TestCase.assertEqual(secondNotificationCount, 1);
+
+        TestCase.assertThrows(function() {
+            realm.addListener('invalid', function() {});
+        });
+
+        realm.addListener('change', function() {
+            throw new Error('error');
+        });
+
+        TestCase.assertThrows(function() {
+            realm.write(function() {});
+        });
     },
 });