From 2db87b1bbbd91ba62bf970edc1e90e76ce1302b0 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 27 Aug 2013 11:20:16 +0200 Subject: [PATCH 1/6] Adding .idea folder to .gitignore. --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index c1d40255..1b834e50 100644 --- a/.gitignore +++ b/.gitignore @@ -7,3 +7,4 @@ test/config.js test/coverage.html lib-cov/ *~ +.idea \ No newline at end of file From 6ad0bbe85c130f2d309eab0375367b8f37d034e4 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 27 Aug 2013 11:45:50 +0200 Subject: [PATCH 2/6] Add .complete() method to Promise and find-chain --- lib/ChainFind.js | 7 +++++++ lib/Promise.js | 13 ++++++++++--- test/integration/model-find-chain.js | 13 +++++++++++++ 3 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/ChainFind.js b/lib/ChainFind.js index 7edaba9e..c01889fd 100644 --- a/lib/ChainFind.js +++ b/lib/ChainFind.js @@ -126,6 +126,13 @@ function ChainFind(Model, opts) { } return promise.fail(cb); }, + complete: function (cb) { + if (!promise) { + promise = new Promise(); + promise.handle(this.all); + } + return promise.complete(cb); + }, all: function (cb) { opts.driver.find(opts.only, opts.table, opts.conditions, { limit : opts.limit, diff --git a/lib/Promise.js b/lib/Promise.js index 0008adea..5e2d6780 100644 --- a/lib/Promise.js +++ b/lib/Promise.js @@ -3,18 +3,21 @@ exports.Promise = Promise; function Promise(opts) { opts = opts || {}; - var success_cb = opts.success || null; - var fail_cb = opts.fail || null; + var success_cb = opts.success || null; + var fail_cb = opts.fail || null; + var complete_cb = opts.complete || null; return { handle: function (promise) { promise(function (err) { + if (err) { if (fail_cb) fail_cb(err); + if (complete_cb) complete_cb(err, null); } else { var args = Array.prototype.slice.call(arguments, 1); - if (success_cb) success_cb.apply(null, args); + if (complete_cb) complete_cb.apply(null, args); } }); }, @@ -25,6 +28,10 @@ function Promise(opts) { fail: function (cb) { fail_cb = cb; return this; + }, + complete: function (cb) { + complete_cb = cb; + return this; } }; } diff --git a/test/integration/model-find-chain.js b/test/integration/model-find-chain.js index 53b335da..0bfc1cc4 100644 --- a/test/integration/model-find-chain.js +++ b/test/integration/model-find-chain.js @@ -489,4 +489,17 @@ describe("Model.find() chaining", function() { }); }); }); + + describe(".complete()", function () { + before(setup()); + + it("should return a Promise with .complete() method", function (done) { + Person.find().complete(function (err, people) { + should(Array.isArray(people)); + should.equal(err, null); + + return done(); + }); + }) + }); }); From 6e31dd52868aaa1c757ea0e6d6a79cc92605e163 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 27 Aug 2013 12:03:56 +0200 Subject: [PATCH 3/6] =?UTF-8?q?I=20really=20should=20have=20testet=20this?= =?UTF-8?q?=20before=20opening=20a=20pull=20request=E2=80=A6?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- lib/Promise.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/Promise.js b/lib/Promise.js index 5e2d6780..76a3b4cc 100644 --- a/lib/Promise.js +++ b/lib/Promise.js @@ -12,12 +12,12 @@ function Promise(opts) { promise(function (err) { if (err) { - if (fail_cb) fail_cb(err); if (complete_cb) complete_cb(err, null); + if (fail_cb) fail_cb(err); } else { - var args = Array.prototype.slice.call(arguments, 1); - if (success_cb) success_cb.apply(null, args); + var args = Array.prototype.slice.call(arguments); if (complete_cb) complete_cb.apply(null, args); + if (success_cb) success_cb.apply(null, args.slice(1)); } }); }, From f40bb01161df139633491cfb5fd7ab0d81a6eed0 Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 27 Aug 2013 14:21:27 +0200 Subject: [PATCH 4/6] Added .then() method to find-chain and Promise and some more tests. It's also possible to add more than one specific callback. For example, there could be two or any combination of .success() or two .fail() callbacks. --- lib/ChainFind.js | 7 ++ lib/Promise.js | 38 ++++++--- test/integration/model-find-chain.js | 116 +++++++++++++++++++++++++-- 3 files changed, 143 insertions(+), 18 deletions(-) diff --git a/lib/ChainFind.js b/lib/ChainFind.js index c01889fd..58c5ff0e 100644 --- a/lib/ChainFind.js +++ b/lib/ChainFind.js @@ -133,6 +133,13 @@ function ChainFind(Model, opts) { } return promise.complete(cb); }, + then: function (success_cb, error_cb) { + if (!promise) { + promise = new Promise(); + promise.handle(this.all); + } + return promise.then(success_cb, error_cb); + }, all: function (cb) { opts.driver.find(opts.only, opts.table, opts.conditions, { limit : opts.limit, diff --git a/lib/Promise.js b/lib/Promise.js index 76a3b4cc..81f5228d 100644 --- a/lib/Promise.js +++ b/lib/Promise.js @@ -3,34 +3,52 @@ exports.Promise = Promise; function Promise(opts) { opts = opts || {}; - var success_cb = opts.success || null; - var fail_cb = opts.fail || null; - var complete_cb = opts.complete || null; + var success_cb = []; + var fail_cb = []; + var complete_cb = []; + + success_cb.push(opts.success); + fail_cb.push(opts.fail); + complete_cb.push(opts.complete); return { handle: function (promise) { promise(function (err) { if (err) { - if (complete_cb) complete_cb(err, null); - if (fail_cb) fail_cb(err); + for (var i = 0, len = complete_cb.length; i < len; i++) { + if (complete_cb[i]) complete_cb[i](err, null); + } + for (var i = 0, len = fail_cb.length; i < len; i++) { + if (fail_cb[i]) fail_cb[i](err); + } } else { var args = Array.prototype.slice.call(arguments); - if (complete_cb) complete_cb.apply(null, args); - if (success_cb) success_cb.apply(null, args.slice(1)); + for (var i = 0, len = complete_cb.length; i < len; i++) { + if (complete_cb[i]) complete_cb[i].apply(null, args); + } + args = args.slice(1); + for (var i = 0, len = success_cb.length; i < len; i++) { + if (success_cb[i]) success_cb[i].apply(null, args); + } } }); }, success: function (cb) { - success_cb = cb; + success_cb.push(cb); return this; }, fail: function (cb) { - fail_cb = cb; + fail_cb.push(cb); return this; }, complete: function (cb) { - complete_cb = cb; + complete_cb.push(cb); + return this; + }, + then: function (success, error) { + success_cb.push(success); + fail_cb.push(error); return this; } }; diff --git a/test/integration/model-find-chain.js b/test/integration/model-find-chain.js index 0bfc1cc4..a91fccaa 100644 --- a/test/integration/model-find-chain.js +++ b/test/integration/model-find-chain.js @@ -468,25 +468,61 @@ describe("Model.find() chaining", function() { it("should return a Promise with .fail() method", function (done) { Person.find().success(function (people) { should(Array.isArray(people)); - - return done(); }).fail(function (err) { // never called.. + }).then(function () { + return done() }); }); + + it("should allow multiple invocations", function (done) { + var invocations = 0; + + Person.find().success(function (people) { + invocations++; + should(Array.isArray(people)); + }).success(function (people) { + invocations++; + should(Array.isArray(people)); + }); + + setTimeout(function () { + should.equal(invocations, 2); + return done(); + }, 500); + }); }); describe(".fail()", function () { before(setup()); - it("should return a Promise with .success() method", function (done) { - Person.find().fail(function (err) { - // never called.. + it("should return a Promise with .fail() method", function (done) { + Person.find({ notExistingField: "" }).fail(function (err) { + should.exist(err); }).success(function (people) { - should(Array.isArray(people)); + // never called.. + }).then(function () { + // never called.. + }, function () { + return done() + }); + }); - return done(); + it("should allow multiple invocations", function (done) { + var invocations = 0; + + Person.find({ notExistingField: "" }).fail(function (err) { + invocations++; + should.exist(err); + }).fail(function (err) { + invocations++; + should.exist(err); }); + + setTimeout(function () { + should.equal(invocations, 2); + return done(); + }, 500); }); }); @@ -497,9 +533,73 @@ describe("Model.find() chaining", function() { Person.find().complete(function (err, people) { should(Array.isArray(people)); should.equal(err, null); + }).success(function (people) { + should(Array.isArray(people)); + }).fail(function (err) { + should.equal(err, null); + }).then(function () { + return done() + }); + }); + + it("should allow multiple invocations", function (done) { + var invocations = 0; + + Person.find().complete(function (err, people) { + invocations++; + should(Array.isArray(people)); + should.equal(err, null); + }).complete(function (err, people) { + invocations++; + should(Array.isArray(people)); + should.equal(err, null); + }); + + setTimeout(function () { + should.equal(invocations, 2); + return done(); + }, 500); + }); + }); + + describe(".then()", function () { + before(setup()); + + it("should return a Promise with .then() method", function (done) { + Person.find().complete(function (err, people) { + should(Array.isArray(people)); + should.equal(err, null); + }).then(function () { + return done() + }); + }); + it("should call the error callback on error", function (done) { + Person.find({ notExistingField: "" }).fail(function (err) { + should.exist(err); + }).then(function () { + // never called.. + }, function (err) { + should.exist(err); return done(); }); - }) + }); + + it("should allow multiple invocations", function (done) { + var invocations = 0; + + Person.find().then(function (people) { + invocations++; + should(Array.isArray(people)); + }).then(function (people) { + invocations++; + should(Array.isArray(people)); + }); + + setTimeout(function () { + should.equal(invocations, 2); + return done(); + }, 500); + }); }); }); From 1964467dbd6494ee907d1728a29d8011090e1feb Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 27 Aug 2013 14:34:48 +0200 Subject: [PATCH 5/6] Generating errors with not existing fields does not work with mongodb. --- test/integration/model-find-chain.js | 36 +++------------------------- 1 file changed, 3 insertions(+), 33 deletions(-) diff --git a/test/integration/model-find-chain.js b/test/integration/model-find-chain.js index a91fccaa..95b68106 100644 --- a/test/integration/model-find-chain.js +++ b/test/integration/model-find-chain.js @@ -497,32 +497,13 @@ describe("Model.find() chaining", function() { before(setup()); it("should return a Promise with .fail() method", function (done) { - Person.find({ notExistingField: "" }).fail(function (err) { - should.exist(err); + Person.find().fail(function (err) { + // never called.. }).success(function (people) { // never called.. }).then(function () { - // never called.. - }, function () { - return done() - }); - }); - - it("should allow multiple invocations", function (done) { - var invocations = 0; - - Person.find({ notExistingField: "" }).fail(function (err) { - invocations++; - should.exist(err); - }).fail(function (err) { - invocations++; - should.exist(err); - }); - - setTimeout(function () { - should.equal(invocations, 2); return done(); - }, 500); + }); }); }); @@ -574,17 +555,6 @@ describe("Model.find() chaining", function() { }); }); - it("should call the error callback on error", function (done) { - Person.find({ notExistingField: "" }).fail(function (err) { - should.exist(err); - }).then(function () { - // never called.. - }, function (err) { - should.exist(err); - return done(); - }); - }); - it("should allow multiple invocations", function (done) { var invocations = 0; From b2ff09491db00ff7459996c888b01c5ddb71e50b Mon Sep 17 00:00:00 2001 From: Philipp Date: Tue, 27 Aug 2013 22:08:02 +0200 Subject: [PATCH 6/6] Changed Promise API a bit. - Renamed .success and .complete methods. - Left .fail tests commented out, because I could not find a hook to create errors in find chains. --- lib/ChainFind.js | 12 ++-- lib/Promise.js | 76 +++++++++++++++--------- test/integration/model-find-chain.js | 88 +++++++++++++++++++--------- 3 files changed, 114 insertions(+), 62 deletions(-) diff --git a/lib/ChainFind.js b/lib/ChainFind.js index 58c5ff0e..70a374d9 100644 --- a/lib/ChainFind.js +++ b/lib/ChainFind.js @@ -112,12 +112,12 @@ function ChainFind(Model, opts) { run: function (cb) { return this.all(cb); }, - success: function (cb) { + done: function (cb) { if (!promise) { promise = new Promise(); promise.handle(this.all); } - return promise.success(cb); + return promise.done(cb); }, fail: function (cb) { if (!promise) { @@ -126,19 +126,19 @@ function ChainFind(Model, opts) { } return promise.fail(cb); }, - complete: function (cb) { + always: function (cb) { if (!promise) { promise = new Promise(); promise.handle(this.all); } - return promise.complete(cb); + return promise.always(cb); }, - then: function (success_cb, error_cb) { + then: function (done_cb, fail_cb) { if (!promise) { promise = new Promise(); promise.handle(this.all); } - return promise.then(success_cb, error_cb); + return promise.then(done_cb, fail_cb); }, all: function (cb) { opts.driver.find(opts.only, opts.table, opts.conditions, { diff --git a/lib/Promise.js b/lib/Promise.js index 81f5228d..3a92ba0f 100644 --- a/lib/Promise.js +++ b/lib/Promise.js @@ -3,52 +3,74 @@ exports.Promise = Promise; function Promise(opts) { opts = opts || {}; - var success_cb = []; - var fail_cb = []; - var complete_cb = []; + var resolved = false; + var revoked = false; + var run = false; - success_cb.push(opts.success); + var done_cb = []; + var fail_cb = []; + var always_cb = []; + + done_cb.push(opts.done); fail_cb.push(opts.fail); - complete_cb.push(opts.complete); + always_cb.push(opts.always); + + var revoke = function (err) { + + if (run) return; + + revoked = run = true; + + for (var i = 0, len = always_cb.length; i < len; i++) { + if (always_cb[i]) always_cb[i](err, null); + } + for (var i = 0, len = fail_cb.length; i < len; i++) { + if (fail_cb[i]) fail_cb[i](err); + } + }; + + var resolve = function () { + + if (run) return; + + resolved = run = true; + + var args = Array.prototype.slice.call(arguments); + for (var i = 0, len = always_cb.length; i < len; i++) { + if (always_cb[i]) always_cb[i].apply(null, args); + } + args = args.slice(1); + for (var i = 0, len = done_cb.length; i < len; i++) { + if (done_cb[i]) done_cb[i].apply(null, args); + } + }; return { handle: function (promise) { - promise(function (err) { + promise(function (err) { if (err) { - for (var i = 0, len = complete_cb.length; i < len; i++) { - if (complete_cb[i]) complete_cb[i](err, null); - } - for (var i = 0, len = fail_cb.length; i < len; i++) { - if (fail_cb[i]) fail_cb[i](err); - } + revoke(err); } else { - var args = Array.prototype.slice.call(arguments); - for (var i = 0, len = complete_cb.length; i < len; i++) { - if (complete_cb[i]) complete_cb[i].apply(null, args); - } - args = args.slice(1); - for (var i = 0, len = success_cb.length; i < len; i++) { - if (success_cb[i]) success_cb[i].apply(null, args); - } + resolve.apply(null, arguments); } }); }, - success: function (cb) { - success_cb.push(cb); + done: function (cb) { + done_cb.push(cb); return this; }, fail: function (cb) { fail_cb.push(cb); return this; }, - complete: function (cb) { - complete_cb.push(cb); + always: function (cb) { + always_cb.push(cb); return this; }, - then: function (success, error) { - success_cb.push(success); - fail_cb.push(error); + then: function (done, fail) { + done_cb.push(done); + fail_cb.push(fail); return this; } }; diff --git a/test/integration/model-find-chain.js b/test/integration/model-find-chain.js index 95b68106..0763a1f2 100644 --- a/test/integration/model-find-chain.js +++ b/test/integration/model-find-chain.js @@ -462,15 +462,15 @@ describe("Model.find() chaining", function() { }); }); - describe(".success()", function () { + describe(".done()", function () { before(setup()); - it("should return a Promise with .fail() method", function (done) { - Person.find().success(function (people) { + it("should return a Promise with .done() method", function (done) { + Person.find().done(function (people) { should(Array.isArray(people)); }).fail(function (err) { // never called.. - }).then(function () { + }).always(function () { return done() }); }); @@ -478,10 +478,10 @@ describe("Model.find() chaining", function() { it("should allow multiple invocations", function (done) { var invocations = 0; - Person.find().success(function (people) { + Person.find().done(function (people) { invocations++; should(Array.isArray(people)); - }).success(function (people) { + }).done(function (people) { invocations++; should(Array.isArray(people)); }); @@ -496,41 +496,53 @@ describe("Model.find() chaining", function() { describe(".fail()", function () { before(setup()); - it("should return a Promise with .fail() method", function (done) { - Person.find().fail(function (err) { - // never called.. - }).success(function (people) { - // never called.. - }).then(function () { - return done(); - }); - }); +// it("should return a Promise with .fail() method that calles the callback on query failure", function (done) { +// Person.find().fail(function (err) { +// should.exist(err); +// }).done(function (people) { +// // never called... +// }).always(function () { +// return done(); +// }); +// }); + +// it("should allow multiple invocations", function (done) { +// var invocations = 0; +// +// Person.find().fail(function (err) { +// invocations++; +// should.exist(err); +// }).fail(function (err) { +// invocations++; +// should.exist(err); +// }); +// +// setTimeout(function () { +// should.equal(invocations, 2); +// return done(); +// }, 500); +// }); }); - describe(".complete()", function () { + describe(".always()", function () { before(setup()); - it("should return a Promise with .complete() method", function (done) { - Person.find().complete(function (err, people) { - should(Array.isArray(people)); - should.equal(err, null); - }).success(function (people) { + it("should return a Promise with .always() method", function (done) { + Person.find().always(function (err, people) { should(Array.isArray(people)); - }).fail(function (err) { should.equal(err, null); - }).then(function () { - return done() + return done(); }); }); it("should allow multiple invocations", function (done) { var invocations = 0; - Person.find().complete(function (err, people) { + Person.find().always(function (err, people) { invocations++; should(Array.isArray(people)); should.equal(err, null); - }).complete(function (err, people) { + }).always(function (err, people) { invocations++; should(Array.isArray(people)); should.equal(err, null); @@ -547,23 +559,41 @@ describe("Model.find() chaining", function() { before(setup()); it("should return a Promise with .then() method", function (done) { - Person.find().complete(function (err, people) { + Person.find().then(function (people) { + should(Array.isArray(people)); + }, function (err) { + // never called + }).always(function (err, people) { should(Array.isArray(people)); should.equal(err, null); - }).then(function () { - return done() + return done(); }); }); +// it("should call the provided fail callback on failure", function (done) { +// Person.find().remove().then(function (people) { +// // never called +// }, function (err) { +// should.exist(err); +// }).always(function (err, people) { +// should.exist(err); +// return done(); +// }); +// }); + it("should allow multiple invocations", function (done) { var invocations = 0; Person.find().then(function (people) { invocations++; should(Array.isArray(people)); + }, function (err) { + // never called }).then(function (people) { invocations++; should(Array.isArray(people)); + }, function (err) { + // never called }); setTimeout(function () {