Correctly resolve methods

package.json

@@ -1,7 +1,7 @@
 {
-  "name": "asteroid",
-  "version": "2.0.3",
-  "description": "Alternative Meteor client",
+  "name": "backlog-asteroid",
+  "version": "2.1.0",
+  "description": "Temporary clone of asteroid",
   "main": "lib/asteroid.js",
   "scripts": {
     "build": "babel src --out-dir lib",

src/base-mixins/methods.js

@@ -12,18 +12,50 @@
 */
 
 export function apply (method, params) {
+    let onResult = params && params[params.length - 1];
+    if (typeof onResult === "function") {
+        params.pop();
+    } else {
+        onResult = undefined;
+    }
     return new Promise((resolve, reject) => {
         const id = this.ddp.method(method, params);
-        this.methods.cache[id] = {resolve, reject};
+        this.methods.cache[id] = {
+            resolve,
+            reject,
+            onResult,
+        };
     });
 }
 
 export function call (method, ...params) {
     return this.apply(method, params);
 }
 
+export function updateMethod (id) {
+    const method = this.methods.cache[id];
+    // if there was no previous `result` event, there is no result
+    // stored that we can use to resolve
+    // since every method invocation will have one `updated` and one
+    // `result` event, we now wait until the `result` event occurs
+    if (!method.hasOwnProperty("result")) {
+        this.methods.cache[id].updated = true;
+        return;
+    }
+    method.resolve(method.result);
+    delete this.methods.cache[id];
+}
+
 /*
 *   Init method
+*   The method lifecycle contains exactly one `result` event and one `updated` event.
+*   - Once the Method has finished running on the server, it sends a `result` message.
+*   - If the method has updates that are relevant to the client's subscriptions,
+*   the server sends those relevant updates, and emits an `updated` event afterward.
+*   - if there are no relevant data updates, the `updated` event is emitted before
+*   the `results` event (for whatever reason...)
+*   See the meteor guide for more information about the method lifecycle
+*   https://guide.meteor.com/methods.html#call-lifecycle
 */
 
 export function init () {
@@ -34,9 +66,22 @@ export function init () {
         const method = this.methods.cache[id];
         if (error) {
             method.reject(error);
-        } else {
+        } else if (method.updated) {
+            // only resolve if there was a previous `updated` event
             method.resolve(result);
+        } else {
+            // since there was no previous `update` event we have to cache the
+            // result and resolve the promise with this result when the
+            // `updated` event is emitted
+            this.methods.cache[id].result = result;
+            if (this.methods.cache[id].onResult) {
+                this.methods.cache[id].onResult(result);
+            }
+            return;
         }
         delete this.methods.cache[id];
     });
+    this.ddp.on("updated", ({methods}) => {
+        methods.forEach(updateMethod.bind(this));
+    });
 }

src/common/login-method.js

@@ -8,11 +8,11 @@ export function onLogin ({id, token}) {
         .then(() => id);
 }
 
-export function onLogout () {
+export function onLogout (err) {
     this.userId = null;
     this.loggedIn = false;
     return multiStorage.del(this.endpoint + "__login_token__")
-        .then(this.emit.bind(this, "loggedOut"))
+        .then(this.emit.bind(this, "loggedOut", err))
         .then(() => null);
 }
 

test/unit/base-mixins/methods.js

@@ -11,9 +11,75 @@ chai.use(sinonChai);
 
 describe("`methods` mixin", () => {
 
+    describe("`updated` event handle", () => {
+
+        it("resolves the promise with the value from `result`", () => {
+            const result = {
+                foo: "bar"
+            };
+            const instance = {
+                ddp: new EventEmitter()
+            };
+            init.call(instance);
+            const resolve = sinon.spy();
+            const reject = sinon.spy();
+            instance.methods.cache["id"] = {resolve, reject};
+            instance.ddp.emit("result", {
+                id: "id",
+                result
+            });
+            instance.ddp.emit("updated", {
+                methods: ["id"]
+            });
+            expect(resolve).to.have.been.calledWith(result);
+            expect(reject).to.have.callCount(0);
+        });
+
+        it("should not resolve when there was no previous `result` event", () => {
+            const instance = {
+                ddp: new EventEmitter()
+            };
+            init.call(instance);
+            const resolve = sinon.spy();
+            const reject = sinon.spy();
+            instance.methods.cache["id"] = {resolve, reject};
+            instance.ddp.emit("updated", {
+                methods: ["id"]
+            });
+            expect(resolve).to.have.callCount(0);
+            expect(reject).to.have.callCount(0);
+        });
+
+    });
+
     describe("`result` event handler", () => {
 
-        it("resolves the promise in the `methods.cache` if no errors occurred", () => {
+        it("does resolve if no errors occurred and there was a previous `update` event", () => {
+            const result = {
+                foo: "bar"
+            };
+            const instance = {
+                ddp: new EventEmitter()
+            };
+            init.call(instance);
+            const resolve = sinon.spy();
+            const reject = sinon.spy();
+            instance.methods.cache["id"] = {resolve, reject};
+            instance.ddp.emit("updated", {
+                methods: ["id"]
+            });
+            instance.ddp.emit("result", {
+                id: "id",
+                result
+            });
+            expect(resolve).to.have.been.calledWith(result);
+            expect(reject).to.have.callCount(0);
+        });
+
+        it("does not resolve if no errors occurred", () => {
+            const result = {
+                foo: "bar"
+            };
             const instance = {
                 ddp: new EventEmitter()
             };
@@ -23,13 +89,13 @@ describe("`methods` mixin", () => {
             instance.methods.cache["id"] = {resolve, reject};
             instance.ddp.emit("result", {
                 id: "id",
-                result: {}
+                result
             });
-            expect(resolve).to.have.been.calledWith({});
+            expect(resolve).to.have.callCount(0);
             expect(reject).to.have.callCount(0);
         });
 
-        it("rejects the promise in the `methods.cache` if errors occurred", () => {
+        it("rejects if errors occurred", () => {
             const instance = {
                 ddp: new EventEmitter()
             };
@@ -45,6 +111,26 @@ describe("`methods` mixin", () => {
             expect(reject).to.have.been.calledWith({});
         });
 
+        it("calls onResult callback", () => {
+            const onResult = sinon.spy();
+            const result = {foo: "bar"};
+            const instance = {ddp: new EventEmitter()};
+            init.call(instance);
+            const resolve = sinon.spy();
+            const reject = sinon.spy();
+            instance.methods.cache["id"] = {
+                resolve,
+                reject,
+                onResult,
+            };
+            instance.ddp.emit("result", {
+                id: "id",
+                result
+            });
+            expect(onResult.calledOnce).to.equal(true);
+            expect(onResult.firstCall.args[0]).to.deep.equal(result);
+        });
+
     });
 
     describe("`apply` method", () => {
@@ -53,7 +139,10 @@ describe("`methods` mixin", () => {
             const instance = {
                 ddp: {
                     method: sinon.spy()
-                }
+                },
+                methods: {
+                    cache: {},
+                },
             };
             const ret = apply.call(instance);
             expect(ret).to.be.an.instanceOf(Promise);
@@ -64,12 +153,29 @@ describe("`methods` mixin", () => {
             const instance = {
                 ddp: {
                     method: sinon.spy()
-                }
+                },
+                methods: {
+                    cache: {},
+                },
             };
             apply.call(instance, "method", [{}]);
             expect(instance.ddp.method).to.have.been.calledWith("method", [{}]);
         });
 
+        it("should store onResult callback", () => {
+            const onResult = () => "bar";
+            const instance = {
+                ddp: {
+                    method: sinon.spy(() => "method-id-1")
+                },
+                methods: {
+                    cache: {},
+                },
+            };
+            apply.call(instance, "method", ["foo", onResult]);
+            expect(instance.methods.cache["method-id-1"].onResult).to.equal(onResult);
+        });
+
     });
 
     describe("`call` method", () => {