Sequencing async operations followed by onResult call

Question

Fiddle: http://jsfiddle.net/smartdev101/eLxxpjp3/

Inside asyncAction function call, a promise has been created to sequence two async operations, getRecords and getTotal followed by a final call to onResult(data) on success or onFail(info) in case of exception.

how to get onResult call upon completion of two async operations?

asyncAction: function(url, params, resultFunction, faultFunction) {
    puremvc.asyncproxy.AsyncProxy.prototype.asyncAction.call(this, resultFunction, faultFunction);

    if(!params.id) { //get master
        var queryString = this.url.parse(url, true).query;
        var offset = queryString.offset ? Number(queryString.offset) : 0;
        var limit = queryString.limit ? Number(queryString.limit) : 20;

        var data = {rows: null, total: null};

        var self = this;
        this.getRecords(data, offset, limit)
        .then(function(data){return self.getTotal(data)})
        //.fail(function(error){self.onFault(error)})
        .done(function(data){self.onResult(data)})

    } else { //get detail
        this.getDetail(params.id);
    }
},

getRecords: function(data, offset, limit) {
    console.log('get records');
    var defer = this.q.defer();
    this.connection.query("SELECT title, name, company FROM speaker LIMIT ? OFFSET ?", [limit, offset], function(error, rows, fields){
        console.log('get records done');
        data.rows = rows;
        defer.resolve(data);
        //defer.reject("earlier");
    });
    return defer.promise;
},

getTotal: function(data) {
    console.log('get total');
    var defer = this.q.defer();

    this.connection.query("SELECT * FROM speaker", function(error, rows, fields) { //SQL_CALC_FOUND_ROWS later
        data.total = rows.length;
        console.log('get total done');
        defer.resolve(data);
        //defer.reject("just like that");
    });

    return defer.promise;
},

onResult: function(data) {
    console.log('on result');
    puremvc.asyncproxy.AsyncProxy.prototype.onResult.call(this, data);
},

onFault: function(info) {
    puremvc.asyncproxy.AsyncProxy.prototype.onFault.call(this, info);
}

please help to get to onResult(data), I'm getting data as null — user2727195, Oct 24 '14 at 17:28
This is a bad way to use promises. Your async operations such as `getRecords()` should create, return and resolve their own promise, not operate on a defer passed in. You can then return that promise from a `.then()` handler in order to chain operations sequentially. Plus, a given defer/promise can ONLY be used once so you can't use the same defer with both `getRecords()` and `getTotal()`, again why they should create and return their own promises. — jfriend00, Oct 24 '14 at 19:31
@jfriend00 appreciate your critique, that's what I'm looking for, a good way to use promises in the current context, all I want is `data{}` to be passed down the chain, `getRecords` populating it's `rows` fields, then `getTotal` populating it's `total` fields, and a call to `onResult(data)` upon completion, can you please rewrite the above while utilizing best practices. — user2727195, Oct 24 '14 at 19:38
updated my fiddle as per your suggestions, but I'm not there yet, please have a look at http://jsfiddle.net/smartdev101/eLxxpjp3/ — user2727195, Oct 24 '14 at 20:05
a promise cannot resolve or reject after it has been resolved or rejected. — Kevin B, Oct 24 '14 at 21:15
sorry @KevinB fiddle has the latest code, I'm editing my question as well — user2727195, Oct 24 '14 at 21:18
@KevinB can you please advise with the fail handler, calling defer.reject calls the fail method as well as done method, it has to be either fail or done, please advise — user2727195, Oct 24 '14 at 21:30
What i was referring to is how you're using the same deferred object in both getRecords and getTotal. Once it is resolved or rejected by getRecords, it can't be resolved or rejected again by getTotals. — Kevin B, Oct 24 '14 at 22:02
@KevinB got you, can you please help me with the latest fiddle — user2727195, Oct 25 '14 at 02:00
I've also promisified `getConnection` under my own answer below, please comment if you see if things could be done in a better way. — user2727195, Oct 26 '14 at 00:54

user2727195 · Answer 1 · 2014-10-26T00:53:10.500

0

finally after struggling a lot, here's the solution, but I need couple of more things, calling defer.reject in any function doesn't halt the process, even done is executed

this post helped a lot. https://coderwall.com/p/ijy61g

Edit 2- Used bind as a result of suggestions - the problem is onResult gets called irrespective of failure, so I had to do if(data) check which I don't like, would have been awesome if promises had some kind of final .success (counterpart of .fail) function.

Edit 3 - added this.onResult at the end of then chain though it doesn't return any promise, is it violating any specs?

Edit 4 - promisified getConnection

getConnection: function() {
    var defer = this.q.defer();

    var mysql = require("mysql");
    var pool = mysql.createPool({
        host: common.Config.mySQLHost,
        user: common.Config.mySQLUsername,
        password: common.Config.mySQLPassword,
        database: common.Config.mySQLDatabase
    });

    pool.getConnection(function(error, connection){
        if(error) { 
            defer.reject(error)
        } else {
            defer.resolve(connection);
        }
    });
    return defer.promise;
},

asyncAction: function(url, params, resultFunction, faultFunction) {

    if(!params.id) { //get master
        var queryString = this.url.parse(url, true).query;
        var offset = queryString.offset ? Number(queryString.offset) : 0;
        var limit = queryString.limit ? Number(queryString.limit) : 20;

        var data = {rows: null, total: null};

        var self = this;
        this.getConnection()
        .then(function(connection){return self.getRecords(data, offset, limit, connection)})
        .then(function(value){return self.getTotal(value.data, value.connection)})
        .then(function(value){self.onResult(value.data, value.connection)})
        .fail(function(value){self.onFault(value.error, value.connection)})
    } else { //get detail
        this.getDetail(params.id);
    }
},

getRecords: function(data, offset, limit, connection) {
    var defer = this.q.defer();
    connection.query("SELECT title, name, company FROM speaker LIMIT ? OFFSET ?", [limit, offset], function(error, rows, fields){
        if(error) {
            defer.reject({error:error, connection:connection});
        } else {
            data.rows = rows;
            defer.resolve({data: data, connection:connection});
        }
    });
    return defer.promise;
},

getTotal: function(data, connection) {
    var defer = this.q.defer();
    connection.query("SELECT count(*) AS total FROM speaker", function(error, rows, fields) { 
        if(error) {
            defer.reject({error:error, connection:connection});
        } else {
            data.total = rows[0].total;
            defer.resolve({connection:connection, data:data});
        }
    });
    return defer.promise;
},

onResult: function(data, connection) {
    console.log(data);
    connection.release();
},

onFault: function(info, connection) {
     console.log(info)
    connection.release();
}

edited Oct 26 '14 at 00:53

answered Oct 24 '14 at 21:03

user2727195

7,122
17
70
118

You should instead promisify `connection.query` it would make your life much easier. – Benjamin Gruenbaum Oct 25 '14 at 10:21
Will the promise chain in `asyncAction()` not simplify to `this.getRecords({}, offset, limit).then(this.getTotal).fail(this.onFault).done(this.onResult);`? `data` and `that` can disappear. – Roamer-1888 Oct 25 '14 at 15:26
@Roamer-1888 well context was important under which getRecords, getTotal, onFault and onResult are running, I used closures but now it's simplified to .call(this, param) instead, agree? – user2727195 Oct 25 '14 at 19:46
can anyone suggest me how error handling can be done in above scenario, what I want is to be onFault to be called and no call to onResult, but right now both are getting called even if the promise rejects – user2727195 Oct 25 '14 at 19:48
Waaaaah! (a) Context is not an issue; there's no need to re-bind the same `this` as the `this` that's already there, and (b) You can't bind in a parameter at that stage. Ask yourself, where would `error` come from and is `data` the correct `data`? Try your version and mine - see which one works. – Roamer-1888 Oct 25 '14 at 20:20
where's your version? – user2727195 Oct 25 '14 at 21:12
can you please write as an answer – user2727195 Oct 25 '14 at 21:23
My opening comment above - that version. – Roamer-1888 Oct 25 '14 at 21:30
I tried, with this version, the context `this` gets changed, inside `getTotal`, the `this` becomes a global context, therefore, `this.q`, and `this.connection` fails, however, I changed your version a bit, used bind statements and it works. `this.getTotal.bind(this)` – user2727195 Oct 25 '14 at 23:09
@Roamer-1888 In your version `getTotal` runs only after `getRecords` is done, doesn't it? You should start them at the same time. (Or does it do that?, I don't know `q`.) – Rudie Oct 25 '14 at 23:28
@Rudie these are chained async calls, first `getResult` has to succeed for `getTotal` to be called and then finally `onResult` if everything goes fine else `onFault` – user2727195 Oct 25 '14 at 23:33
@BenjaminGruenbaum have promisified `getConnection`, any comments. though my then functions got really populated, I learned that resolve let's you pass only one argument (value) and also I have to pass `connection` down the stream so I could call `release` at the end, looking for best possible way to do things in the above context – user2727195 Oct 26 '14 at 00:50
BG suggested that you promisify `connection.query` to make life much easier. You have done something different and made life much harder. – Roamer-1888 Oct 26 '14 at 05:40
@Roamer-1888 I realized now, but that'd be too much micromanagement since I've 2 async functions in a sequence and I want their resolves/rejects, plus `getConnection` is going to be under commons as I've several proxy files accessing it, also it gets the benefits of using connection pools – user2727195 Oct 26 '14 at 07:40

Evil Buck · Accepted Answer · 2014-10-25T23:30:00.953

0

It seems like this could be simplified. It seems like you just want these async operations to be performed synchronously. If you return the promise, the original promise in the chain will become the new promise. The final fail will catch the first promise to be rejected.

asyncAction: function(url, params, resultFunction, faultFunction) {
    puremvc.asyncproxy.AsyncProxy.prototype.asyncAction.call(this, resultFunction, faultFunction);

    if(!params.id) { //get master
        var queryString = this.url.parse(url, true).query;
        var offset = queryString.offset ? Number(queryString.offset) : 0;
        var limit = queryString.limit ? Number(queryString.limit) : 20;

        var data = {rows: null, total: null};

        var self = this;
        this.getRecords(data, offset, limit)
        .then(function(data) {
          return self.getTotal(data);
        })
        .then(this.onResult.bind(this))
        .fail(this.onFault);
    } else { //get detail
        this.getDetail(params.id);
    }
},

getRecords: function(data, offset, limit) {
    console.log('get records');
    var defer = this.q.defer();
    this.connection.query("SELECT title, name, company FROM speaker LIMIT ? OFFSET ?", [limit, offset], function(error, rows, fields){
        console.log('get records done');
        data.rows = rows;
        defer.resolve(data);
        //defer.reject("earlier");
    });
    return defer.promise;
},

getTotal: function(data) {
    console.log('get total');
    var defer = this.q.defer();

    this.connection.query("SELECT * FROM speaker", function(error, rows, fields) { //SQL_CALC_FOUND_ROWS later
        data.total = rows.length;
        console.log('get total done');
        defer.resolve(data);
        //defer.reject("just like that");
    });

    return defer.promise;
},

onResult: function(data) {
    console.log('on result');
    puremvc.asyncproxy.AsyncProxy.prototype.onResult.call(this, data);
},

edited Oct 25 '14 at 23:30

answered Oct 25 '14 at 19:55

Evil Buck

1,068
7
20

`return self.onResult` is confusing, onResult is not returning anything, it's a standalone call to be called if all promises succeed. onResult triggers the callback to original caller who called asyncAction in the first place on success. please modify the answer to suit the need – user2727195 Oct 25 '14 at 21:16
question, I've also seen `done` somewhere, what does it do? `.done(function(data){self.onResult('done ' + data)})`, how's it gonna work in case of errors – user2727195 Oct 25 '14 at 22:53
Can you please have a look at my answer below, see edit 2 and my comments in the code above `onResult`, your approach is good but if you can get me a solution for the onResult comments, I'd accept your answer. – user2727195 Oct 25 '14 at 23:22
actually you've the answer, `.then(this.onResult.bind(this))` is there any problem with using that since it's not returning any promise, what the specs says – user2727195 Oct 25 '14 at 23:26
I've promisified `getConnection`, can you please comment for any cleanup, efficient way. I had to revert to closure based approach since resolve only allows on param to be passed, so I had to create an object in resolve and then open it up for the next function call, also have to pass connection down for `release` calls. Please comment for a best strategy in the given context if you see something not efficient – user2727195 Oct 26 '14 at 00:52
the `done` function is passed whatever the deferred is resolved with. So assuming `Q`, `var testDefer = Q.defer(); testDefer.promise.then(function(data) { console.log(data); }; testDefer.resolve('tada'); // will console.log('tada')` – Evil Buck Oct 26 '14 at 03:36
I don't see a getConnection implemented here. Can you show your code? – Evil Buck Oct 26 '14 at 03:40
the `getConnection` is in my answer below – user2727195 Oct 26 '14 at 04:15

score 0 · Answer 3 · answered Oct 26 '14 at 06:33

User2727195, here's my version in full, based on the original, unedited code posted in your own answer.

There's no attemtpt to improve the logic/flow, just to improve the syntax. As such, it is only as good as your answer was at that time. To get everything working properly, you will need to apply your own later ideas and other suggestions offered since then.

As such, this is not an independent answer and should not be selected. If it starts to attract negative votes, then I will delete it so trawl through for ideas while you can.

asyncAction: function(url, params) {
    if(!params.id) { //get master
        var queryString = this.url.parse(url, true).query;
        this.getRecords({}, Number(queryString.offset || 0), Number(queryString.limit || 20))
            .then(this.getTotal.bind(this))
            .fail(this.onFault.bind(this))
            .done(this.onResult.bind(this));
        } else { //get detail
        this.getDetail(params.id);
    }
},
connectionQueryPromisifier = function() {
    // This is a promisifying adaptor for connection.query .
    // In your own version, you will probably choose to rewrite connection.query rather than use an adapter.
    var args = Array.prototype.slice.call(arguments).concat(function(error, rows, fields) {
        if(error) { defer.reject(error); }
        else { defer.resolve({ rows:rows, fields:fields }); }
    }),
        defer = this.q.defer();
    this.connection.query.apply(this, args);
    return defer.promise;
},
getRecords: function(data, offset, limit) {
    //Here, you take advantage of having promisified connection.query .
    return this.connectionQueryPromisifier("SELECT title, name, company FROM speaker LIMIT ? OFFSET ?", [limit, offset]).then(function(obj) {
        data.rows = obj.rows;
    });
},
getTotal: function(data) {
    //Here, you take advantage of having promisified connection.query .
    return this.connectionQueryPromisifier("SELECT * FROM speaker").then(function(obj) {
        data.total = obj.rows.length;
    });
},
onResult: function(data) {
    console.log('on result');
},
onFault: function(info) {
    console.log('onFault');
}

looks interesting, I'll be reviewing it – user2727195 Oct 26 '14 at 07:42 — user2727195, Oct 26 '14 at 07:42

Sequencing async operations followed by onResult call

3 Answers3

Linked