From 93f61f979d4fbdd342276bb32b9c151f2ff2adbc Mon Sep 17 00:00:00 2001 From: philippspo Date: Sun, 30 Oct 2016 20:43:00 +0100 Subject: [PATCH 1/4] feat(methods): resolve on updated event with result from result event --- src/base-mixins/methods.js | 9 ++++++++- test/unit/base-mixins/methods.js | 30 ++++++++++++++++++++++++++++-- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/src/base-mixins/methods.js b/src/base-mixins/methods.js index b4aa0e2..ec3288b 100644 --- a/src/base-mixins/methods.js +++ b/src/base-mixins/methods.js @@ -31,11 +31,18 @@ export function init () { cache: {} }; this.ddp.on("result", ({id, error, result}) => { + const method = this.methods.cache[id]; + if (error) { + method.reject(error); + } + this.methods.cache[id].result = result; + }); + this.ddp.on("updated", ({id, error}) => { const method = this.methods.cache[id]; if (error) { method.reject(error); } else { - method.resolve(result); + method.resolve(method.result); } delete this.methods.cache[id]; }); diff --git a/test/unit/base-mixins/methods.js b/test/unit/base-mixins/methods.js index 692ae91..6390af6 100644 --- a/test/unit/base-mixins/methods.js +++ b/test/unit/base-mixins/methods.js @@ -11,9 +11,35 @@ chai.use(sinonChai); describe("`methods` mixin", () => { + describe("`updated` event handle", () => { + + it("resolves the promise with the returned value", () => { + 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", { + id: "id" + }); + expect(resolve).to.have.been.calledWith(result); + expect(reject).to.have.callCount(0); + }); + + }); + describe("`result` event handler", () => { - it("resolves the promise in the `methods.cache` if no errors occurred", () => { + it("does not resolve the promise in the `methods.cache` if no errors occurred", () => { const instance = { ddp: new EventEmitter() }; @@ -25,7 +51,7 @@ describe("`methods` mixin", () => { id: "id", result: {} }); - expect(resolve).to.have.been.calledWith({}); + expect(resolve).to.have.callCount(0); expect(reject).to.have.callCount(0); }); From 1e2f3a267d30e8ca965c24eadf1dff46d2f68bed Mon Sep 17 00:00:00 2001 From: philippspo Date: Sun, 30 Oct 2016 23:01:50 +0100 Subject: [PATCH 2/4] fix(methods): handle cases where updated event is emitted before result event --- src/base-mixins/methods.js | 41 +++++++++++++++++++++----- src/common/login-method.js | 4 +-- test/unit/base-mixins/methods.js | 50 ++++++++++++++++++++++++++++---- 3 files changed, 80 insertions(+), 15 deletions(-) diff --git a/src/base-mixins/methods.js b/src/base-mixins/methods.js index ec3288b..288351d 100644 --- a/src/base-mixins/methods.js +++ b/src/base-mixins/methods.js @@ -22,8 +22,30 @@ 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,16 +56,19 @@ export function init () { const method = this.methods.cache[id]; if (error) { method.reject(error); - } - this.methods.cache[id].result = result; - }); - this.ddp.on("updated", ({id, error}) => { - const method = this.methods.cache[id]; - if (error) { - method.reject(error); + } else if (method.updated) { + // only resolve if there was a previous `updated` event + method.resolve(result); } else { - method.resolve(method.result); + // 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; + return; } delete this.methods.cache[id]; }); + this.ddp.on("updated", ({methods}) => { + methods.forEach(updateMethod.bind(this)); + }); } diff --git a/src/common/login-method.js b/src/common/login-method.js index 90b2491..695694f 100644 --- a/src/common/login-method.js +++ b/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); } diff --git a/test/unit/base-mixins/methods.js b/test/unit/base-mixins/methods.js index 6390af6..1fd7731 100644 --- a/test/unit/base-mixins/methods.js +++ b/test/unit/base-mixins/methods.js @@ -13,7 +13,7 @@ describe("`methods` mixin", () => { describe("`updated` event handle", () => { - it("resolves the promise with the returned value", () => { + it("resolves the promise with the value from `result`", () => { const result = { foo: "bar" }; @@ -29,17 +29,35 @@ describe("`methods` mixin", () => { result }); instance.ddp.emit("updated", { - id: "id" + 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("does not resolve 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() }; @@ -47,15 +65,37 @@ describe("`methods` mixin", () => { 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: {} + 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() + }; + init.call(instance); + const resolve = sinon.spy(); + const reject = sinon.spy(); + instance.methods.cache["id"] = {resolve, reject}; + instance.ddp.emit("result", { + id: "id", + result }); 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() }; From cefa80f4b7945ec2cec681523d8dfabb2d4d7b55 Mon Sep 17 00:00:00 2001 From: philippspo Date: Sun, 18 Dec 2016 16:51:09 +0100 Subject: [PATCH 3/4] feat(methods): onResult callback --- package.json | 4 +-- src/base-mixins/methods.js | 15 ++++++++++- test/unit/base-mixins/methods.js | 44 ++++++++++++++++++++++++++++++-- 3 files changed, 58 insertions(+), 5 deletions(-) diff --git a/package.json b/package.json index c63f2fe..48377d0 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { - "name": "asteroid", + "name": "backlog-asteroid", "version": "2.0.3", - "description": "Alternative Meteor client", + "description": "Temporary clone of asteroid", "main": "lib/asteroid.js", "scripts": { "build": "babel src --out-dir lib", diff --git a/src/base-mixins/methods.js b/src/base-mixins/methods.js index 288351d..a12d7c9 100644 --- a/src/base-mixins/methods.js +++ b/src/base-mixins/methods.js @@ -12,9 +12,19 @@ */ 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, + }; }); } @@ -64,6 +74,9 @@ export function init () { // 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]; diff --git a/test/unit/base-mixins/methods.js b/test/unit/base-mixins/methods.js index 1fd7731..d14d687 100644 --- a/test/unit/base-mixins/methods.js +++ b/test/unit/base-mixins/methods.js @@ -111,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", () => { @@ -119,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); @@ -130,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", () => { From 0d5fc8816148b12306717c8b760eff43f41a1be2 Mon Sep 17 00:00:00 2001 From: philippspo Date: Sun, 8 Jan 2017 13:30:43 +0100 Subject: [PATCH 4/4] chore(version): bump version to 2.1.0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 48377d0..f991de0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "backlog-asteroid", - "version": "2.0.3", + "version": "2.1.0", "description": "Temporary clone of asteroid", "main": "lib/asteroid.js", "scripts": {